From e6f1d613b683802c6fe7a7d5155fa07724dd4f02 Mon Sep 17 00:00:00 2001 From: Brin Shadewater Date: Sat, 1 Aug 2026 02:24:01 -0700 Subject: [PATCH] fix(discord): leave voice channels before cancelling the bot task `DiscordAdapter.disconnect()` cancelled the bot task before tearing down voice clients. `leave_voice_channel()` ends in `await vc.disconnect()`, and discord.py sends a voice state update over the main gateway websocket and then waits for the voice socket to close. The bot task is the loop running that gateway connection, so cancelling it first left the handshake with no transport: it could never complete and blocked until the caller's shutdown timeout fired. The effect was a fixed ~5s penalty on every shutdown with a voice connection open, ending in "discord disconnect timed out after 5.0s - forcing continue", with the voice disconnect abandoned rather than completed. Measured on a live gateway with a voice connection open in both cases: before: timed out after 5.0s, all adapters disconnected at +5.29s after: discord disconnected (0.12s), all adapters disconnected at +0.46s Moving the voice-cleanup loop above `_cancel_bot_task()` preserves the zombie-client protection its comment describes: the bot task is still cancelled before `client.close()`, just after voice teardown rather than before it. Voice teardown is the one step that still requires a live gateway. Adds a regression test asserting the ordering. It fails on the previous ordering at index 1 with `cancel_bot_task != leave_voice_channel:111`. Fixes #76044 --- plugins/platforms/discord/adapter.py | 19 ++++++++---- tests/gateway/test_voice_command.py | 43 ++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index 56cb90a949107..c7c3036f75f9e 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -1773,6 +1773,19 @@ class DiscordAdapter(BasePlatformAdapter): # Cancel the liveness probe first so it can't fire a spurious fatal # error / reconnect while we're intentionally tearing the adapter down. await self._cancel_liveness_task() + # Clean up all active voice connections *before* cancelling the bot task. + # leave_voice_channel() ends in `await vc.disconnect()`, and discord.py's + # VoiceClient.disconnect() sends a voice state update over the main + # gateway websocket and then waits for the voice socket to close. The + # bot task is the loop running that gateway connection, so cancelling it + # first leaves the handshake with no transport: it can never complete and + # blocks until the caller's shutdown timeout fires. + for guild_id in list(self._voice_clients.keys()): + try: + await self.leave_voice_channel(guild_id) + except Exception as e: # pragma: no cover - defensive logging + logger.debug("[%s] Error leaving voice channel %s: %s", self.name, guild_id, e) + # Cancel the bot task before closing the client. If connect() timed out # and returned False, the background client.start() task may still be # running; calling client.close() alone is not enough to stop it because @@ -1780,12 +1793,6 @@ class DiscordAdapter(BasePlatformAdapter): # WebSocket handshake is in flight. Explicitly cancelling the task here # ensures the zombie client cannot receive or dispatch any further events. await self._cancel_bot_task() - # Clean up all active voice connections before closing the client - for guild_id in list(self._voice_clients.keys()): - try: - await self.leave_voice_channel(guild_id) - except Exception as e: # pragma: no cover - defensive logging - logger.debug("[%s] Error leaving voice channel %s: %s", self.name, guild_id, e) if self._client: try: diff --git a/tests/gateway/test_voice_command.py b/tests/gateway/test_voice_command.py index 4a988a6e036ac..6f1a1d7e3d5db 100644 --- a/tests/gateway/test_voice_command.py +++ b/tests/gateway/test_voice_command.py @@ -767,6 +767,49 @@ class TestDiscordVoiceChannelMethods: adapter._is_allowed_user.assert_called_once_with("42", guild=adapter._client.get_guild(111), is_dm=False) + @pytest.mark.asyncio + async def test_disconnect_leaves_voice_before_cancelling_bot_task(self): + """Voice must be torn down while the gateway websocket is still alive. + + VoiceClient.disconnect() sends a voice state update over the main gateway + connection and waits for the voice socket to close. The bot task is the + loop running that connection, so cancelling it first strands the + handshake and the disconnect blocks until the caller's shutdown timeout. + """ + adapter = self._make_adapter() + events = [] + + async def cancel_liveness_task(): + events.append("cancel_liveness_task") + + async def cancel_bot_task(): + events.append("cancel_bot_task") + + async def leave_voice_channel(guild_id): + events.append(f"leave_voice_channel:{guild_id}") + + async def close(): + events.append("close_client") + + adapter._cancel_liveness_task = cancel_liveness_task + adapter._cancel_bot_task = cancel_bot_task + adapter.leave_voice_channel = leave_voice_channel + adapter._client.close = close + adapter._voice_clients[111] = MagicMock() + adapter._ready_event = MagicMock() + adapter._post_connect_task = None + adapter._missed_message_backfill_task = None + + await adapter.disconnect() + + assert events == [ + "cancel_liveness_task", + "leave_voice_channel:111", + "cancel_bot_task", + "close_client", + ] + + @pytest.mark.asyncio async def test_get_user_voice_channel_success(self): adapter = self._make_adapter()