Fixed VoiceChannel.ConnectAsync nullref and race conditions

src/Discord.Net/WebSocket/DiscordSocketClient.cs

@@ -1402,12 +1402,17 @@ namespace Discord.WebSocket
                                                 {
                                                     before = guild.GetVoiceState(data.UserId)?.Clone() ?? new VoiceState(null, null, false, false, false);
                                                     after = guild.AddOrUpdateVoiceState(data, DataStore);
+                                                    if (data.UserId == _currentUser.Id)
+                                                    {
+                                                        var _ = guild.FinishJoinAudioChannel().ConfigureAwait(false);
+                                                    }
                                                 }
                                                 else
                                                 {
                                                     before = guild.RemoveVoiceState(data.UserId) ?? new VoiceState(null, null, false, false, false);
                                                     after = new VoiceState(null, data);
                                                 }
+
                                                 user = guild.GetUser(data.UserId);
                                             }
                                             else
@@ -1460,7 +1465,7 @@ namespace Discord.WebSocket
                                     if (guild != null)
                                     {
                                         string endpoint = data.Endpoint.Substring(0, data.Endpoint.LastIndexOf(':'));
-                                        var _ = guild.ConnectAudio(_nextAudioId++, endpoint, data.Token).ConfigureAwait(false);
+                                        var _ = guild.FinishConnectAudio(_nextAudioId++, endpoint, data.Token).ConfigureAwait(false);
                                     }
                                     else
                                     {

src/Discord.Net/WebSocket/Entities/Channels/SocketVoiceChannel.cs

@@ -41,13 +41,10 @@ namespace Discord
             var audioMode = Discord.AudioMode;
             if (audioMode == AudioMode.Disabled)
                 throw new InvalidOperationException($"Audio is not enabled on this client, {nameof(DiscordSocketConfig.AudioMode)} in {nameof(DiscordSocketConfig)} must be set.");
-
-            await Discord.ApiClient.SendVoiceStateUpdateAsync(Guild.Id, Id,
-                (audioMode & AudioMode.Incoming) == 0, 
+                        
+            return await Guild.ConnectAudioAsync(Id,
+                (audioMode & AudioMode.Incoming) == 0,
                 (audioMode & AudioMode.Outgoing) == 0).ConfigureAwait(false);
-
-            await Guild.AudioConnectPromise.ConfigureAwait(false);
-            return Guild.AudioClient;
         }
 
         public SocketVoiceChannel Clone() => MemberwiseClone() as SocketVoiceChannel;

src/Discord.Net/WebSocket/Entities/Guilds/SocketGuild.cs

@@ -24,7 +24,8 @@ namespace Discord
         internal override bool IsAttached => true;
 
         private readonly SemaphoreSlim _audioLock;
-        private TaskCompletionSource<bool> _syncPromise, _downloaderPromise, _audioConnectPromise;
+        private TaskCompletionSource<bool> _syncPromise, _downloaderPromise;
+        private TaskCompletionSource<AudioClient> _audioConnectPromise;
         private ConcurrentHashSet<ulong> _channels;
         private ConcurrentDictionary<ulong, SocketGuildUser> _members;
         private ConcurrentDictionary<ulong, VoiceState> _voiceStates;
@@ -39,7 +40,6 @@ namespace Discord
         public bool IsSynced => _syncPromise.Task.IsCompleted;
         public Task SyncPromise => _syncPromise.Task;
         public Task DownloaderPromise => _downloaderPromise.Task;
-        public Task AudioConnectPromise => _audioConnectPromise.Task;
 
         public new DiscordSocketClient Discord => base.Discord as DiscordSocketClient;
         public SocketGuildUser CurrentUser => GetUser(Discord.CurrentUser.Id);
@@ -261,38 +261,99 @@ namespace Discord
             return null;
         }
 
-        public async Task ConnectAudio(int id, string url, string token)
+        public async Task<IAudioClient> ConnectAudioAsync(ulong channelId, bool selfDeaf, bool selfMute)
+        {
+            try
+            {
+                TaskCompletionSource<AudioClient> promise;
+
+                await _audioLock.WaitAsync().ConfigureAwait(false);
+                try
+                {
+                    await DisconnectAudioInternalAsync().ConfigureAwait(false);
+                    promise = new TaskCompletionSource<AudioClient>();
+                    _audioConnectPromise = promise;
+                    await Discord.ApiClient.SendVoiceStateUpdateAsync(Id, channelId, selfDeaf, selfMute).ConfigureAwait(false);
+                }
+                finally
+                {
+                    _audioLock.Release();
+                }
+
+                var timeoutTask = Task.Delay(15000);
+                if (await Task.WhenAny(promise.Task, timeoutTask) == timeoutTask)
+                    throw new TimeoutException();
+                return await promise.Task.ConfigureAwait(false);
+            }
+            catch (Exception)
+            {
+                await DisconnectAudioInternalAsync().ConfigureAwait(false);
+                throw;
+            }
+        }
+        public async Task DisconnectAudioAsync(AudioClient client = null)
         {
-            AudioClient audioClient;
             await _audioLock.WaitAsync().ConfigureAwait(false);
+            try
+            {
+                await DisconnectAudioInternalAsync(client).ConfigureAwait(false);
+            }
+            finally
+            {
+                _audioLock.Release();
+            }
+        }
+        private async Task DisconnectAudioInternalAsync(AudioClient client = null)
+        {
+            var oldClient = AudioClient;
+            if (oldClient != null)
+            {
+                if (client == null || oldClient == client)
+                {
+                    _audioConnectPromise?.TrySetCanceledAsync(); //Cancel any previous audio connection
+                    _audioConnectPromise = null;
+                }
+                if (oldClient == client)
+                {
+                    AudioClient = null;
+                    await oldClient.DisconnectAsync().ConfigureAwait(false);
+                }
+            }
+        }
+        public async Task FinishConnectAudio(int id, string url, string token)
+        {
             var voiceState = GetVoiceState(CurrentUser.Id).Value;
+
+            await _audioLock.WaitAsync().ConfigureAwait(false);
             try
             {
-                audioClient = AudioClient;
-                if (audioClient == null)
+                if (AudioClient == null)
                 {
-                    audioClient = new AudioClient(this, id);
+                    var audioClient = new AudioClient(this, id);
                     audioClient.Disconnected += async ex =>
                     {
                         await _audioLock.WaitAsync().ConfigureAwait(false);
                         try
                         {
-                            if (ex != null)
+                            if (AudioClient == audioClient) //Only reconnect if we're still assigned as this guild's audio client
                             {
-                                //Reconnect if we still have channel info.
-                                //TODO: Is this threadsafe? Could channel data be deleted before we access it?
-                                var voiceState2 = GetVoiceState(CurrentUser.Id);
-                                if (voiceState2.HasValue)
+                                if (ex != null)
                                 {
-                                    var voiceChannelId = voiceState2.Value.VoiceChannel?.Id;
-                                    if (voiceChannelId != null)
-                                        await Discord.ApiClient.SendVoiceStateUpdateAsync(Id, voiceChannelId, voiceState2.Value.IsSelfDeafened, voiceState2.Value.IsSelfMuted);
+                                    //Reconnect if we still have channel info.
+                                    //TODO: Is this threadsafe? Could channel data be deleted before we access it?
+                                    var voiceState2 = GetVoiceState(CurrentUser.Id);
+                                    if (voiceState2.HasValue)
+                                    {
+                                        var voiceChannelId = voiceState2.Value.VoiceChannel?.Id;
+                                        if (voiceChannelId != null)
+                                            await Discord.ApiClient.SendVoiceStateUpdateAsync(Id, voiceChannelId, voiceState2.Value.IsSelfDeafened, voiceState2.Value.IsSelfMuted);
+                                    }
+                                }
+                                else
+                                {
+                                    try { AudioClient.Dispose(); } catch { }
+                                    AudioClient = null;
                                 }
-                            }
-                            else
-                            {
-                                try { AudioClient.Dispose(); } catch { }
-                                AudioClient = null;
                             }
                         }
                         finally
@@ -302,20 +363,34 @@ namespace Discord
                     };
                     AudioClient = audioClient;
                 }
+                await AudioClient.ConnectAsync(url, CurrentUser.Id, voiceState.VoiceSessionId, token).ConfigureAwait(false);
+                await _audioConnectPromise.TrySetResultAsync(AudioClient).ConfigureAwait(false);
+            }
+            catch (OperationCanceledException)
+            {
+                await DisconnectAudioAsync();
+            }
+            catch (Exception e)
+            {
+                await _audioConnectPromise.SetExceptionAsync(e).ConfigureAwait(false);
+                await DisconnectAudioAsync();
             }
             finally
             {
                 _audioLock.Release();
             }
-
+        }
+        public async Task FinishJoinAudioChannel()
+        {
+            await _audioLock.WaitAsync().ConfigureAwait(false);
             try
             {
-                await audioClient.ConnectAsync(url, CurrentUser.Id, voiceState.VoiceSessionId, token).ConfigureAwait(false);
-                await _audioConnectPromise.TrySetResultAsync(true).ConfigureAwait(false);
+                if (AudioClient != null)
+                    await _audioConnectPromise.TrySetResultAsync(AudioClient).ConfigureAwait(false);
             }
-            catch(Exception e)
+            finally
             {
-                await _audioConnectPromise.SetExceptionAsync(e).ConfigureAwait(false);
+                _audioLock.Release();
             }
         }