Changed audio connection requests (#457)

* New audio connection system to counter major race conditions
* Remove connection requests from queue when guild is deleted
* Fixed possible NPE on disconnect requests with no channels
* Removed setDaemon from audio thread group
* Added functionality to deal with possible racecondition related to moving a connection during it being opened.

.gitignore

@@ -14,6 +14,7 @@ src/test/
 # gradle
 /build/
 /.gradle/
+/out/
 
 # eclipse files
 .classpath

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

@@ -137,6 +137,7 @@ public void ready()
             JDAImpl api = (JDAImpl) getJDA();
             api.getEventManager().handle(new ExceptionEvent(api, throwable, true));
         });
+        readyThread.setDaemon(true);
         readyThread.setName(threadIdentifier + " Ready Thread");
         readyThread.start();
     }
@@ -423,6 +424,7 @@ else if (couldReceive)
                 JDAImpl api = (JDAImpl) getJDA();
                 api.getEventManager().handle(new ExceptionEvent(api, throwable, true));
             });
+            receiveThread.setDaemon(true);
             receiveThread.setName(threadIdentifier + " Receiving Thread");
             receiveThread.start();
         }
@@ -440,6 +442,7 @@ private synchronized void setupCombinedExecutor()
             combinedAudioExecutor = Executors.newSingleThreadScheduledExecutor((task) ->
             {
                 final Thread t = new Thread(AudioManagerImpl.AUDIO_THREADS, task, threadIdentifier + " Combined Thread");
+                t.setDaemon(true);
                 t.setUncaughtExceptionHandler((thread, throwable) ->
                 {
                     LOG.log(throwable);

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

@@ -17,7 +17,6 @@
 package net.dv8tion.jda.core.audio;
 
 import com.neovisionaries.ws.client.*;
-import net.dv8tion.jda.core.WebSocketCode;
 import net.dv8tion.jda.core.audio.hooks.ConnectionListener;
 import net.dv8tion.jda.core.audio.hooks.ConnectionStatus;
 import net.dv8tion.jda.core.entities.Guild;
@@ -444,18 +443,11 @@ public synchronized void close(ConnectionStatus closeStatus)
                 && closeStatus != ConnectionStatus.AUDIO_REGION_CHANGE) //Already handled.
         {
             manager.setQueuedAudioConnection(disconnectedChannel);
-            api.getClient().queueAudioConnect(disconnectedChannel, true);
+            api.getClient().queueAudioReconnect(disconnectedChannel);
         }
         else if (closeStatus != ConnectionStatus.AUDIO_REGION_CHANGE)
         {
-            JSONObject closeFrame = new JSONObject()
-                .put("op", WebSocketCode.VOICE_STATE)
-                .put("d", new JSONObject()
-                    .put("guild_id", guild.getId())
-                    .put("channel_id", JSONObject.NULL)
-                    .put("self_mute", false)
-                    .put("self_deaf", false));
-            api.getClient().send(closeFrame.toString());
+            api.getClient().queueAudioDisconnect(guild);
         }
     }
 
@@ -648,7 +640,9 @@ public KeepAliveThreadFactory(JDAImpl api)
         @Override
         public Thread newThread(Runnable r)
         {
-            return new Thread(AudioManagerImpl.AUDIO_THREADS, r, identifier + " - Thread " + threadCount.getAndIncrement());
+            final Thread t = new Thread(AudioManagerImpl.AUDIO_THREADS, r, identifier + " - Thread " + threadCount.getAndIncrement());
+            t.setDaemon(true);
+            return t;
         }
     }
 }

src/main/java/net/dv8tion/jda/core/audio/ConnectionRequest.java

@@ -0,0 +1,77 @@
+/*
+ *     Copyright 2015-2017 Austin Keener & Michael Ritter & Florian Spieß
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.dv8tion.jda.core.audio;
+
+import net.dv8tion.jda.core.entities.Guild;
+import net.dv8tion.jda.core.entities.VoiceChannel;
+
+public class ConnectionRequest
+{
+    protected final long guildId;
+    protected long nextAttemptEpoch;
+    protected ConnectionStage stage;
+    protected VoiceChannel channel;
+
+    public ConnectionRequest(Guild guild)
+    {
+        this.stage = ConnectionStage.DISCONNECT;
+        this.guildId = guild.getIdLong();
+    }
+
+    public ConnectionRequest(VoiceChannel channel, ConnectionStage stage)
+    {
+        this.channel = channel;
+        this.guildId = channel.getGuild().getIdLong();
+        this.stage = stage;
+        this.nextAttemptEpoch = System.currentTimeMillis();
+    }
+
+    public void setStage(ConnectionStage stage)
+    {
+        this.stage = stage;
+    }
+
+    public void setChannel(VoiceChannel channel)
+    {
+        this.channel = channel;
+    }
+
+    public void setNextAttemptEpoch(long epochMillis)
+    {
+        this.nextAttemptEpoch = epochMillis;
+    }
+
+    public VoiceChannel getChannel()
+    {
+        return channel;
+    }
+
+    public ConnectionStage getStage()
+    {
+        return stage;
+    }
+
+    public long getNextAttemptEpoch()
+    {
+        return nextAttemptEpoch;
+    }
+
+    public long getGuildIdLong()
+    {
+        return guildId;
+    }
+}

src/main/java/net/dv8tion/jda/core/audio/ConnectionStage.java

@@ -0,0 +1,22 @@
+/*
+ *     Copyright 2015-2017 Austin Keener & Michael Ritter & Florian Spieß
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.dv8tion.jda.core.audio;
+
+public enum ConnectionStage
+{
+    CONNECT, RECONNECT, DISCONNECT
+}

src/main/java/net/dv8tion/jda/core/audio/factory/DefaultSendSystem.java

@@ -102,6 +102,7 @@ public void start()
             SimpleLog.getLog("DefaultSendSystem").log(throwable);
             start();
         });
+        sendThread.setDaemon(true);
         sendThread.setName(packetProvider.getIdentifier() + " Sending Thread");
         sendThread.setPriority((Thread.NORM_PRIORITY + Thread.MAX_PRIORITY) / 2);
         sendThread.start();

src/main/java/net/dv8tion/jda/core/handle/GuildDeleteHandler.java

@@ -68,6 +68,7 @@ protected Long handleInternally(JSONObject content)
             return null;
         }
 
+        api.getClient().removeAudioConnection(id);
         final TLongObjectMap<AudioManagerImpl> audioManagerMap = api.getAudioManagerMap();
         synchronized (audioManagerMap)
         {

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

@@ -35,7 +35,11 @@ public VoiceServerUpdateHandler(JDAImpl api)
     protected Long handleInternally(JSONObject content)
     {
         final long guildId = content.getLong("guild_id");
-        api.getClient().getQueuedAudioConnectionMap().remove(guildId);
+        Guild guild = api.getGuildMap().get(guildId);
+        if (guild == null)
+            throw new IllegalArgumentException("Attempted to start audio connection with Guild that doesn't exist! JSON: " + content);
+
+        api.getClient().updateAudioConnection(guildId, guild.getSelfMember().getVoiceState().getChannel());
 
         if (api.getGuildLock().isLocked(guildId))
             return guildId;
@@ -50,9 +54,6 @@ protected Long handleInternally(JSONObject content)
 
         String endpoint = content.getString("endpoint");
         String token = content.getString("token");
-        Guild guild = api.getGuildMap().get(guildId);
-        if (guild == null)
-            throw new IllegalArgumentException("Attempted to start audio connection with Guild that doesn't exist! JSON: " + content);
         String sessionId = guild.getSelfMember().getVoiceState().getSessionId();
         if (sessionId == null)
             throw new IllegalArgumentException("Attempted to create audio connection without having a session ID. Did VOICE_STATE_UPDATED fail?");

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

@@ -124,20 +124,32 @@ private void handleGuildVoiceState(JSONObject content)
             else if (channel == null)
             {
                 oldChannel.getConnectedMembersMap().remove(userId);
+                if (guild.getSelfMember().equals(member))
+                    api.getClient().updateAudioConnection(guildId, null);
                 api.getEventManager().handle(
                         new GuildVoiceLeaveEvent(
                                 api, responseNumber,
                                 member, oldChannel));
             }
             else
             {
-                //If the connect account is the one that is being moved, and this instance of JDA
-                // is connected or attempting to connect, them change the channel we expect to be connected to.
-                if (guild.getSelfMember().equals(member))
+                AudioManagerImpl mng = api.getAudioManagerMap().get(guildId);
+
+                //If the currently connected account is the one that is being moved
+                if (guild.getSelfMember().equals(member) && mng != null)
                 {
-                    AudioManagerImpl mng = api.getAudioManagerMap().get(guildId);
-                    if (mng != null && (mng.isConnected() || mng.isAttemptingToConnect()))
+                    //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())
                         mng.setConnectedChannel(channel);
+
+                    //If we have connected (VOICE_SERVER_UPDATE received and AudioConnection created (actual connection might still be setting up)),
+                    // then we need to stop sending audioOpen/Move requests through the MainWS if the channel
+                    // we have just joined / moved to is the same as the currently queued audioRequest
+                    // (handled by updateAudioConnection)
+                    if (mng.isConnected())
+                        api.getClient().updateAudioConnection(guildId, channel);
+                    //If we are not already connected this will be removed by VOICE_SERVER_UPDATE
                 }
 
                 channel.getConnectedMembersMap().put(userId, member);

src/main/java/net/dv8tion/jda/core/managers/impl/AudioManagerImpl.java

@@ -18,7 +18,6 @@
 import com.sun.jna.Platform;
 import net.dv8tion.jda.core.JDA;
 import net.dv8tion.jda.core.Permission;
-import net.dv8tion.jda.core.WebSocketCode;
 import net.dv8tion.jda.core.audio.AudioConnection;
 import net.dv8tion.jda.core.audio.AudioReceiveHandler;
 import net.dv8tion.jda.core.audio.AudioSendHandler;
@@ -36,24 +35,17 @@
 import net.dv8tion.jda.core.utils.Checks;
 import net.dv8tion.jda.core.utils.NativeUtil;
 import net.dv8tion.jda.core.utils.PermissionUtil;
-import org.json.JSONObject;
 
 import java.io.IOException;
 
 public class AudioManagerImpl implements AudioManager
 {
-    public static final ThreadGroup AUDIO_THREADS;
+    public static final ThreadGroup AUDIO_THREADS = new ThreadGroup("jda-audio");
     //These values are set at the bottom of this file.
     public static boolean AUDIO_SUPPORTED;
     public static String OPUS_LIB_NAME;
     protected static boolean initialized = false;
 
-    static
-    {
-        AUDIO_THREADS = new ThreadGroup("jda-audio");
-        AUDIO_THREADS.setDaemon(true);
-    }
-
     public final Object CONNECTION_LOCK = new Object();
 
     protected final JDAImpl api;
@@ -105,7 +97,7 @@ public void openAudioConnection(VoiceChannel channel)
         {
             //Start establishing connection, joining provided channel
             queuedAudioConnection = channel;
-            api.getClient().queueAudioConnect(channel, false);
+            api.getClient().queueAudioConnect(channel);
         }
         else
         {
@@ -127,7 +119,7 @@ public void openAudioConnection(VoiceChannel channel)
                             "Unable to connect to VoiceChannel due to userlimit! Requires permission VOICE_MOVE_OTHERS to bypass");
             }
 
-            api.getClient().queueAudioConnect(channel, false);
+            api.getClient().queueAudioConnect(channel);
             audioConnection.setChannel(channel);
         }
     }
@@ -142,11 +134,11 @@ public void closeAudioConnection(ConnectionStatus reason)
     {
         synchronized (CONNECTION_LOCK)
         {
-            api.getClient().getQueuedAudioConnectionMap().remove(guild.getIdLong());
             this.queuedAudioConnection = null;
-            if (audioConnection == null)
-                return;
-            this.audioConnection.close(reason);
+            if (audioConnection != null)
+                this.audioConnection.close(reason);
+            else
+                this.api.getClient().queueAudioDisconnect(guild);
             this.audioConnection = null;
         }
     }
@@ -344,15 +336,7 @@ protected void updateVoiceState()
             VoiceChannel channel = isConnected() ? getConnectedChannel() : getQueuedAudioConnection();
 
             //This is technically equivalent to an audio open/move packet.
-            JSONObject voiceStateChange = new JSONObject()
-                    .put("op", WebSocketCode.VOICE_STATE)
-                    .put("d", new JSONObject()
-                            .put("guild_id", guild.getId())
-                            .put("channel_id", channel.getId())
-                            .put("self_mute", isSelfMuted())
-                            .put("self_deaf", isSelfDeafened())
-                    );
-            api.getClient().send(voiceStateChange.toString());
+            api.getClient().queueAudioConnect(channel);
         }
     }