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
This commit is contained in:
parent
d1c6c6b58e
commit
e6f1d613b6
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
Loading…
Reference in New Issue