Improve handling of VOICE_SERVER_UPDATE (#1302)

* Improve handling of VOICE_SERVER_UPDATE
* Fix deadlock in closeAudioConnections
* Fix deadlock in WebSocketSendingThread

src/main/java/net/dv8tion/jda/api/managers/AudioManager.java

@@ -16,6 +16,8 @@
 
 package net.dv8tion.jda.api.managers;
 
+import net.dv8tion.jda.annotations.DeprecatedSince;
+import net.dv8tion.jda.annotations.ForRemoval;
 import net.dv8tion.jda.annotations.Incubating;
 import net.dv8tion.jda.api.JDA;
 import net.dv8tion.jda.api.audio.AudioReceiveHandler;
@@ -183,7 +185,12 @@ default void setSpeakingMode(@Nonnull SpeakingMode... mode)
      * {@link net.dv8tion.jda.api.entities.VoiceChannel VoiceChannel} that JDA is attempting to setup an audio connection to.
      *
      * @return True, if JDA is currently attempting to create an audio connection.
+     *
+     * @deprecated The internals have changed and this is no longer used
      */
+    @Deprecated
+    @ForRemoval
+    @DeprecatedSince("4.2.0")
     boolean isAttemptingToConnect();
 
     /**
@@ -195,8 +202,13 @@ default void setSpeakingMode(@Nonnull SpeakingMode... mode)
      *
      * @return The {@link net.dv8tion.jda.api.entities.VoiceChannel VoiceChannel} that JDA is attempting to create an
      *         audio connection with, or {@code null} if JDA isn't attempting to create a connection.
+     *
+     * @deprecated The internals have changed and this is no longer used
      */
     @Nullable
+    @Deprecated
+    @ForRemoval
+    @DeprecatedSince("4.2.0")
     VoiceChannel getQueuedAudioConnection();
 
     /**

src/main/java/net/dv8tion/jda/internal/JDAImpl.java

@@ -758,15 +758,17 @@ public synchronized void shutdownRequester()
 
     private void closeAudioConnections()
     {
+        List<AudioManagerImpl> managers;
         AbstractCacheView<AudioManager> view = getAudioManagersView();
         try (UnlockHook hook = view.writeLock())
         {
-            TLongObjectMap<AudioManager> map = view.getMap();
-            map.valueCollection().stream()
+            managers = view.stream()
                .map(AudioManagerImpl.class::cast)
-               .forEach(m -> m.closeAudioConnection(ConnectionStatus.SHUTTING_DOWN));
-            map.clear();
+               .collect(Collectors.toList());
+            view.clear();
         }
+
+        managers.forEach(m -> m.closeAudioConnection(ConnectionStatus.SHUTTING_DOWN));
     }
 
     @Override

src/main/java/net/dv8tion/jda/internal/audio/AudioConnection.java

@@ -37,7 +37,6 @@
 import net.dv8tion.jda.internal.managers.AudioManagerImpl;
 import net.dv8tion.jda.internal.utils.IOUtil;
 import net.dv8tion.jda.internal.utils.JDALogger;
-import net.dv8tion.jda.internal.utils.cache.SnowflakeReference;
 import org.slf4j.Logger;
 import tomp2p.opuswrapper.Opus;
 
@@ -70,7 +69,7 @@ public class AudioConnection
     private final AudioWebSocket webSocket;
     private final JDAImpl api;
 
-    private SnowflakeReference<VoiceChannel> channel;
+    private VoiceChannel channel;
     private PointerByReference opusEncoder;
     private ScheduledExecutorService combinedAudioExecutor;
     private IAudioSendSystem sendSystem;
@@ -87,12 +86,10 @@ public class AudioConnection
     private volatile int speakingMode = SpeakingMode.VOICE.getRaw();
     private volatile int silenceCounter = 0;
 
-    public AudioConnection(AudioManagerImpl manager, String endpoint, String sessionId, String token)
+    public AudioConnection(AudioManagerImpl manager, String endpoint, String sessionId, String token, VoiceChannel channel)
     {
-        VoiceChannel channel = Objects.requireNonNull(manager.getQueuedAudioConnection(), "Failed to create AudioConnection without queued channel!");
-
         this.api = (JDAImpl) channel.getJDA();
-        this.channel = new SnowflakeReference<>(channel, api::getVoiceChannelById);
+        this.channel = channel;
         final JDAImpl api = (JDAImpl) channel.getJDA();
         this.threadIdentifier = api.getIdentifierString() + " AudioConnection Guild: " + channel.getGuild().getId();
         this.webSocket = new AudioWebSocket(this, manager.getListenerProxy(), endpoint, channel.getGuild(), sessionId, token, manager.isAutoReconnect());
@@ -149,12 +146,12 @@ public void setQueueTimeout(long queueTimeout)
 
     public VoiceChannel getChannel()
     {
-        return channel.resolve();
+        return channel;
     }
 
     public void setChannel(VoiceChannel channel)
     {
-        this.channel = channel == null ? null : new SnowflakeReference<>(channel, api::getVoiceChannelById);
+        this.channel = channel;
     }
 
     public JDAImpl getJDA()
@@ -279,7 +276,7 @@ protected void updateUserSSRC(int ssrc, long userId)
                 //Different User already existed with this ssrc. What should we do? Just replace? Probably should nuke the old opusDecoder.
                 //Log for now and see if any user report the error.
                 LOG.error("Yeah.. So.. JDA received a UserSSRC update for an ssrc that already had a User set. Inform DV8FromTheWorld.\nChannelId: {} SSRC: {} oldId: {} newId: {}",
-                      channel.resolve().getId(), ssrc, previousId, userId);
+                      channel.getId(), ssrc, previousId, userId);
             }
         }
         else

src/main/java/net/dv8tion/jda/internal/audio/AudioWebSocket.java

@@ -170,9 +170,6 @@ protected void close(final ConnectionStatus closeStatus)
             audioConnection.shutdown();
 
             VoiceChannel disconnectedChannel = manager.getConnectedChannel();
-            if (disconnectedChannel == null)
-                disconnectedChannel = manager.getQueuedAudioConnection();
-
             manager.setAudioConnection(null);
 
             //Verify that it is actually a lost of connection and not due the connected channel being deleted.
@@ -201,7 +198,6 @@ protected void close(final ConnectionStatus closeStatus)
                     LOG.debug("Cannot reconnect due to null voice channel");
                     return;
                 }
-                manager.setQueuedAudioConnection(disconnectedChannel);
                 api.getDirectAudioController().reconnect(disconnectedChannel);
             }
             else if (status == ConnectionStatus.DISCONNECTED_REMOVED_FROM_GUILD)

src/main/java/net/dv8tion/jda/internal/handle/GuildSetupNode.java

@@ -444,19 +444,15 @@ private void updateAudioManagerReference(GuildImpl guild)
             newMng.setConnectionListener(listener);
             newMng.setAutoReconnect(mng.isAutoReconnect());
 
-            if (mng.isConnected() || mng.isAttemptingToConnect())
+            if (mng.isConnected())
             {
-                final long channelId = mng.isConnected()
-                                       ? mng.getConnectedChannel().getIdLong()
-                                       : mng.getQueuedAudioConnection().getIdLong();
+                final long channelId = mng.getConnectedChannel().getIdLong();
 
                 final VoiceChannel channel = api.getVoiceChannelById(channelId);
                 if (channel != null)
                 {
                     if (mng.isConnected())
                         mng.closeAudioConnection(ConnectionStatus.ERROR_CANNOT_RESUME);
-                    //closing old connection in order to reconnect later
-                    newMng.setQueuedAudioConnection(channel);
                 }
                 else
                 {

src/main/java/net/dv8tion/jda/internal/handle/VoiceServerUpdateHandler.java

@@ -17,6 +17,7 @@
 package net.dv8tion.jda.internal.handle;
 
 import net.dv8tion.jda.api.entities.Guild;
+import net.dv8tion.jda.api.entities.VoiceChannel;
 import net.dv8tion.jda.api.hooks.VoiceDispatchInterceptor;
 import net.dv8tion.jda.api.utils.MiscUtil;
 import net.dv8tion.jda.api.utils.data.DataObject;
@@ -66,22 +67,27 @@ protected Long handleInternally(DataObject content)
             return null;
         }
 
-        AudioManagerImpl audioManager = (AudioManagerImpl) guild.getAudioManager();
+        AudioManagerImpl audioManager = (AudioManagerImpl) getJDA().getAudioManagersView().get(guildId);
+        if (audioManager == null)
+        {
+            WebSocketClient.LOG.debug(
+                "Received a VOICE_SERVER_UPDATE but JDA is not currently connected nor attempted to connect " +
+                "to a VoiceChannel. Assuming that this is caused by another client running on this account. " +
+                "Ignoring the event.");
+            return null;
+        }
+
         MiscUtil.locked(audioManager.CONNECTION_LOCK, () ->
         {
             //Synchronized to prevent attempts to close while setting up initial objects.
-            if (audioManager.isConnected())
-                audioManager.prepareForRegionChange();
-            if (!audioManager.isAttemptingToConnect())
+            VoiceChannel target = guild.getSelfMember().getVoiceState().getChannel();
+            if (target == null)
             {
-                WebSocketClient.LOG.debug(
-                    "Received a VOICE_SERVER_UPDATE but JDA is not currently connected nor attempted to connect " +
-                    "to a VoiceChannel. Assuming that this is caused by another client running on this account. " +
-                    "Ignoring the event.");
+                WebSocketClient.LOG.warn("Ignoring VOICE_SERVER_UPDATE for unknown channel");
                 return;
             }
 
-            AudioConnection connection = new AudioConnection(audioManager, endpoint, sessionId, token);
+            AudioConnection connection = new AudioConnection(audioManager, endpoint, sessionId, token, target);
             audioManager.setAudioConnection(connection);
             connection.startConnection();
         });

src/main/java/net/dv8tion/jda/internal/handle/VoiceStateUpdateHandler.java

@@ -166,7 +166,7 @@ else if (channel == null)
                 {
                     //And this instance of JDA is connected or attempting to connect,
                     // then change the channel we expect to be connected to.
-                    if (mng.isConnected() || mng.isAttemptingToConnect())
+                    if (mng.isConnected())
                         mng.setConnectedChannel(channel);
 
                     //If we have connected (VOICE_SERVER_UPDATE received and AudioConnection created (actual connection might still be setting up)),

src/main/java/net/dv8tion/jda/internal/managers/AudioManagerImpl.java

@@ -44,7 +44,6 @@ public class AudioManagerImpl implements AudioManager
 
     protected final ListenerProxy connectionListener = new ListenerProxy();
     protected final GuildImpl guild;
-    protected VoiceChannel queuedAudioConnection = null;
     protected AudioConnection audioConnection = null;
     protected EnumSet<SpeakingMode> speakingModes = EnumSet.of(SpeakingMode.VOICE);
 
@@ -83,26 +82,15 @@ public void openAudioConnection(VoiceChannel channel)
         //if (!self.hasPermission(channel, Permission.VOICE_CONNECT))
         //    throw new InsufficientPermissionException(Permission.VOICE_CONNECT);
 
-        if (audioConnection == null)
-        {
-            checkChannel(channel, self);
-            //Start establishing connection, joining provided channel
-            queuedAudioConnection = channel;
-            getJDA().getDirectAudioController().connect(channel);
-        }
-        else
-        {
-            //Connection is already established, move to specified channel
-
-            //If we are already connected to this VoiceChannel, then do nothing.
-            if (channel.equals(audioConnection.getChannel()))
-                return;
+        //If we are already connected to this VoiceChannel, then do nothing.
+        if (audioConnection != null && channel.equals(audioConnection.getChannel()))
+            return;
 
-            checkChannel(channel, self);
+        checkChannel(channel, self);
 
-            getJDA().getDirectAudioController().connect(channel);
+        getJDA().getDirectAudioController().connect(channel);
+        if (audioConnection != null)
             audioConnection.setChannel(channel);
-        }
     }
 
     private void checkChannel(VoiceChannel channel, Member self)
@@ -141,7 +129,6 @@ public void closeAudioConnection(ConnectionStatus reason)
     {
         MiscUtil.locked(CONNECTION_LOCK, () ->
         {
-            this.queuedAudioConnection = null;
             if (audioConnection != null)
                 this.audioConnection.close(reason);
             else if (reason != ConnectionStatus.DISCONNECTED_REMOVED_FROM_GUILD)
@@ -189,15 +176,17 @@ public GuildImpl getGuild()
     }
 
     @Override
+    @Deprecated
     public boolean isAttemptingToConnect()
     {
-        return queuedAudioConnection != null;
+        return false;
     }
 
     @Override
+    @Deprecated
     public VoiceChannel getQueuedAudioConnection()
     {
-        return queuedAudioConnection;
+        return null;
     }
 
     @Override
@@ -328,30 +317,23 @@ public ConnectionListener getListenerProxy()
 
     public void setAudioConnection(AudioConnection audioConnection)
     {
-        this.audioConnection = audioConnection;
         if (audioConnection == null)
+        {
+            this.audioConnection = null;
             return;
+        }
 
-        this.queuedAudioConnection = null;
+        // This will set the audioConnection to null, which we then immediately override with the new connection
+        if (this.audioConnection != null)
+            closeAudioConnection(ConnectionStatus.AUDIO_REGION_CHANGE);
+        this.audioConnection = audioConnection;
         audioConnection.setSendingHandler(sendHandler);
         audioConnection.setReceivingHandler(receiveHandler);
         audioConnection.setQueueTimeout(queueTimeout);
         audioConnection.setSpeakingMode(speakingModes);
         audioConnection.setSpeakingDelay(speakingDelay);
     }
 
-    public void prepareForRegionChange()
-    {
-        VoiceChannel queuedChannel = audioConnection.getChannel();
-        closeAudioConnection(ConnectionStatus.AUDIO_REGION_CHANGE);
-        this.queuedAudioConnection = queuedChannel;
-    }
-
-    public void setQueuedAudioConnection(VoiceChannel channel)
-    {
-        queuedAudioConnection = channel;
-    }
-
     public void setConnectedChannel(VoiceChannel channel)
     {
         if (audioConnection != null)
@@ -367,10 +349,9 @@ public void setQueueTimeout(long queueTimeout)
 
     protected void updateVoiceState()
     {
-        if (isConnected() || isAttemptingToConnect())
+        VoiceChannel channel = getConnectedChannel();
+        if (channel != null)
         {
-            VoiceChannel channel = isConnected() ? getConnectedChannel() : getQueuedAudioConnection();
-
             //This is technically equivalent to an audio open/move packet.
             getJDA().getDirectAudioController().connect(channel);
         }

src/main/java/net/dv8tion/jda/internal/requests/WebSocketSendingThread.java

@@ -113,9 +113,10 @@ public void run()
             api.setContext();
             attemptedToSend = false;
             needRateLimit = false;
+            // We do this outside of the lock because otherwise we could potentially deadlock here
+            audioRequest = client.getNextAudioConnectRequest();
             queueLock.lockInterruptibly();
 
-            audioRequest = client.getNextAudioConnectRequest();
             chunkRequest = chunkQueue.peek();
             if (chunkRequest != null)
                 handleChunkSync(chunkRequest);