Fix all found race conditions, and dont send on nil channel (#307)

voice.go

@@ -100,11 +100,15 @@ func (v *VoiceConnection) Speaking(b bool) (err error) {
 	v.wsMutex.Lock()
 	err = v.wsConn.WriteJSON(data)
 	v.wsMutex.Unlock()
+
+	v.Lock()
+	defer v.Unlock()
 	if err != nil {
 		v.speaking = false
 		log.Println("Speaking() write json error:", err)
 		return
 	}
+
 	v.speaking = b
 
 	return
@@ -139,9 +143,9 @@ func (v *VoiceConnection) Disconnect() (err error) {
 	// Send a OP4 with a nil channel to disconnect
 	if v.sessionID != "" {
 		data := voiceChannelJoinOp{4, voiceChannelJoinData{&v.GuildID, nil, true, true}}
-		v.wsMutex.Lock()
+		v.session.wsMutex.Lock()
 		err = v.session.wsConn.WriteJSON(data)
-		v.wsMutex.Unlock()
+		v.session.wsMutex.Unlock()
 		v.sessionID = ""
 	}
 
@@ -149,7 +153,10 @@ func (v *VoiceConnection) Disconnect() (err error) {
 	v.Close()
 
 	v.log(LogInformational, "Deleting VoiceConnection %s", v.GuildID)
+
+	v.session.Lock()
 	delete(v.session.VoiceConnections, v.GuildID)
+	v.session.Unlock()
 
 	return
 }
@@ -185,7 +192,9 @@ func (v *VoiceConnection) Close() {
 
 		// To cleanly close a connection, a client should send a close
 		// frame and wait for the server to close the connection.
+		v.wsMutex.Lock()
 		err := v.wsConn.WriteMessage(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, ""))
+		v.wsMutex.Unlock()
 		if err != nil {
 			v.log(LogError, "error closing websocket, %s", err)
 		}
@@ -246,7 +255,10 @@ func (v *VoiceConnection) waitUntilConnected() error {
 
 	i := 0
 	for {
-		if v.Ready {
+		v.RLock()
+		ready := v.Ready
+		v.RUnlock()
+		if ready {
 			return nil
 		}
 
@@ -409,8 +421,6 @@ func (v *VoiceConnection) onEvent(message []byte) {
 			go v.opusReceiver(v.udpConn, v.close, v.OpusRecv)
 		}
 
-		// Send the ready event
-		v.connected <- true
 		return
 
 	case 3: // HEARTBEAT response
@@ -418,6 +428,9 @@ func (v *VoiceConnection) onEvent(message []byte) {
 		return
 
 	case 4: // udp encryption secret key
+		v.Lock()
+		defer v.Unlock()
+
 		v.op4 = voiceOP4{}
 		if err := json.Unmarshal(e.RawData, &v.op4); err != nil {
 			v.log(LogError, "OP4 unmarshall error, %s, %s", err, string(e.RawData))
@@ -648,8 +661,14 @@ func (v *VoiceConnection) opusSender(udpConn *net.UDPConn, close <-chan struct{}
 
 	// VoiceConnection is now ready to receive audio packets
 	// TODO: this needs reviewed as I think there must be a better way.
+	v.Lock()
 	v.Ready = true
-	defer func() { v.Ready = false }()
+	v.Unlock()
+	defer func() {
+		v.Lock()
+		v.Ready = false
+		v.Unlock()
+	}()
 
 	var sequence uint16
 	var timestamp uint32
@@ -678,7 +697,10 @@ func (v *VoiceConnection) opusSender(udpConn *net.UDPConn, close <-chan struct{}
 			// else, continue loop
 		}
 
-		if !v.speaking {
+		v.RLock()
+		speaking := v.speaking
+		v.RUnlock()
+		if !speaking {
 			err := v.Speaking(true)
 			if err != nil {
 				v.log(LogError, "error sending speaking packet, %s", err)
@@ -691,7 +713,9 @@ func (v *VoiceConnection) opusSender(udpConn *net.UDPConn, close <-chan struct{}
 
 		// encrypt the opus data
 		copy(nonce[:], udpHeader)
+		v.RLock()
 		sendbuf := secretbox.Seal(udpHeader, recvbuf, &nonce, &v.op4.SecretKey)
+		v.RUnlock()
 
 		// block here until we're exactly at the right time :)
 		// Then send rtp audio packet to Discord over UDP

wsapi.go

@@ -475,18 +475,24 @@ func (s *Session) ChannelVoiceJoin(gID, cID string, mute, deaf bool) (voice *Voi
 
 	s.log(LogInformational, "called")
 
+	s.RLock()
 	voice, _ = s.VoiceConnections[gID]
+	s.RUnlock()
 
 	if voice == nil {
 		voice = &VoiceConnection{}
+		s.Lock()
 		s.VoiceConnections[gID] = voice
+		s.Unlock()
 	}
 
+	voice.Lock()
 	voice.GuildID = gID
 	voice.ChannelID = cID
 	voice.deaf = deaf
 	voice.mute = mute
 	voice.session = s
+	voice.Unlock()
 
 	// Send the request to Discord that we want to join the voice channel
 	data := voiceChannelJoinOp{4, voiceChannelJoinData{&gID, &cID, mute, deaf}}
@@ -517,7 +523,9 @@ func (s *Session) onVoiceStateUpdate(st *VoiceStateUpdate) {
 	}
 
 	// Check if we have a voice connection to update
+	s.RLock()
 	voice, exists := s.VoiceConnections[st.GuildID]
+	s.RUnlock()
 	if !exists {
 		return
 	}
@@ -528,8 +536,10 @@ func (s *Session) onVoiceStateUpdate(st *VoiceStateUpdate) {
 	}
 
 	// Store the SessionID for later use.
+	voice.Lock()
 	voice.UserID = st.UserID
 	voice.sessionID = st.SessionID
+	voice.Unlock()
 }
 
 // onVoiceServerUpdate handles the Voice Server Update data websocket event.
@@ -541,7 +551,9 @@ func (s *Session) onVoiceServerUpdate(st *VoiceServerUpdate) {
 
 	s.log(LogInformational, "called")
 
+	s.RLock()
 	voice, exists := s.VoiceConnections[st.GuildID]
+	s.RUnlock()
 
 	// If no VoiceConnection exists, just skip this
 	if !exists {
@@ -553,9 +565,11 @@ func (s *Session) onVoiceServerUpdate(st *VoiceServerUpdate) {
 	voice.Close()
 
 	// Store values for later use
+	voice.Lock()
 	voice.token = st.Token
 	voice.endpoint = st.Endpoint
 	voice.GuildID = st.GuildID
+	voice.Unlock()
 
 	// Open a conenction to the voice server
 	err := voice.open()
@@ -645,6 +659,8 @@ func (s *Session) reconnect() {
 				// However, there seems to be cases where something "weird"
 				// happens.  So we're doing this for now just to improve
 				// stability in those edge cases.
+				s.RLock()
+				defer s.RUnlock()
 				for _, v := range s.VoiceConnections {
 
 					s.log(LogInformational, "reconnecting voice connection to guild %s", v.GuildID)
@@ -692,7 +708,9 @@ func (s *Session) Close() (err error) {
 		s.log(LogInformational, "sending close frame")
 		// To cleanly close a connection, a client should send a close
 		// frame and wait for the server to close the connection.
+		s.wsMutex.Lock()
 		err := s.wsConn.WriteMessage(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, ""))
+		s.wsMutex.Unlock()
 		if err != nil {
 			s.log(LogInformational, "error closing websocket, %s", err)
 		}