From c98bd6820e80a1f557520a495494c9f98f50931c Mon Sep 17 00:00:00 2001 From: RogueException Date: Mon, 1 Aug 2016 05:14:22 -0300 Subject: [PATCH] Fixed VoiceChannel.ConnectAsync nullref and race conditions --- .../WebSocket/DiscordSocketClient.cs | 7 +- .../Entities/Channels/SocketVoiceChannel.cs | 9 +- .../WebSocket/Entities/Guilds/SocketGuild.cs | 125 ++++++++++++++---- 3 files changed, 109 insertions(+), 32 deletions(-) diff --git a/src/Discord.Net/WebSocket/DiscordSocketClient.cs b/src/Discord.Net/WebSocket/DiscordSocketClient.cs index 292a909fc..0dca59214 100644 --- a/src/Discord.Net/WebSocket/DiscordSocketClient.cs +++ b/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 { diff --git a/src/Discord.Net/WebSocket/Entities/Channels/SocketVoiceChannel.cs b/src/Discord.Net/WebSocket/Entities/Channels/SocketVoiceChannel.cs index d5be011e9..fbd9d061e 100644 --- a/src/Discord.Net/WebSocket/Entities/Channels/SocketVoiceChannel.cs +++ b/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; diff --git a/src/Discord.Net/WebSocket/Entities/Guilds/SocketGuild.cs b/src/Discord.Net/WebSocket/Entities/Guilds/SocketGuild.cs index e1ed18261..4bf30a725 100644 --- a/src/Discord.Net/WebSocket/Entities/Guilds/SocketGuild.cs +++ b/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 _syncPromise, _downloaderPromise, _audioConnectPromise; + private TaskCompletionSource _syncPromise, _downloaderPromise; + private TaskCompletionSource _audioConnectPromise; private ConcurrentHashSet _channels; private ConcurrentDictionary _members; private ConcurrentDictionary _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 ConnectAudioAsync(ulong channelId, bool selfDeaf, bool selfMute) + { + try + { + TaskCompletionSource promise; + + await _audioLock.WaitAsync().ConfigureAwait(false); + try + { + await DisconnectAudioInternalAsync().ConfigureAwait(false); + promise = new TaskCompletionSource(); + _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(); } }