mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-28 14:10:37 +00:00
5841c8e9b7
* fix(channels): make Feishu and DingTalk transports stoppable in-process
FeishuChannel.stop() and DingTalkChannel.stop() could not end their SDK
threads. Both SDKs run on a thread-private event loop that only returned
on a fatal error: lark-oapi's ws.Client.start() parks forever in
run_until_complete(_select()) and has no public stop, and the channel
never kept the client handle; dingtalk-stream's start_forever() is a
while-True loop that also catches CancelledError, and the disconnect()
the channel called does not exist on the SDK. So every stop joined for
5s, raised ChannelStopTimeout, and ChannelService retained the instance
while the platform connection stayed alive on the old process. That
burns the full budget on shutdown and on POST /api/channels/{name}/restart,
and it blocks the per-provider leader lease: a demoted leader could not
release the transport before a peer took over.
Both channels now publish the SDK thread's loop and client under a lock;
stop() schedules a shutdown on that loop and then joins. Feishu disarms
lark's _auto_reconnect before closing the socket and stopping the loop,
because start() otherwise swallows the interrupted run_until_complete in
its reconnect branch and dials again. DingTalk drives its own copy of the
SDK connect loop that re-checks the running flag before every dial, runs
the blocking ticket request off the loop, cancels keepalive with the
socket, and logs the endpoint rather than the ticket-bearing payload. A
shared shutdown_private_loop cancels the SDK's leftover tasks with a
bounded drain and closes the loop; a stop requested before the client
exists never dials. ChannelStopTimeout now only fires for a thread that
genuinely wedges inside an SDK call. Telegram already stopped this way.
Tests: a fake lark ws.Client with the SDK's start() shape (including the
reconnect branch that swallows a bare loop stop), a real local websockets
server that observes the DingTalk close, the stop-before-client race for
both, and a blocking-io anchor that stop() ends a live private loop
without blocking the Gateway loop. All eight were red on main.
* docs(changelog): link the stoppable-transports entry to #6798
* fix(channels): release the socket on a stalled close and bound the DingTalk ticket request
Review follow-ups on #6798 (Totoro-qaq):
Feishu: the bounded _disconnect() wait cancelled websockets' close before
its own close-timeout abort ran, and lark clears _conn in its finally, so
a peer that never answers the close frame left the socket to garbage
collection. Worse, a stop landing while _connect() still held lark's
_lock snapshotted _conn as None. _shutdown_ws now settles the in-flight
dial by taking that lock under one shared close deadline, snapshots the
connection, runs _disconnect() under the same deadline, and aborts the
transport itself (abort_connection_transport in base.py) when the
handshake did not finish, yielding once so connection_lost runs before
loop.stop(). DingTalk's websocket.close() path gets the same abort.
DingTalk: the SDK's open_connection() calls requests.post with no
timeout, and running it in an executor let a stalled ticket endpoint
block interpreter exit (executor workers are joined at shutdown). The
channel now sends the same request through httpx on the stream loop
under a 30s deadline, so stop() cancels it and no worker thread lingers.
Tests drive the real SDK clients against loopback peers: a WebSocket
peer that completes the handshake and never ACKs the close (both
channels assert the connection's socket reports fileno() == -1 right
after stop(); red with the abort disabled), a real websockets server for
the clean path, and a loopback ticket server (ok / 500 / never responds)
with a no-lingering-non-daemon-thread check and a request-shape check.
137 lines
4.6 KiB
Python
137 lines
4.6 KiB
Python
"""Regression anchor: channel ``stop()`` must not block the event loop on SDK teardown.
|
|
|
|
``stop()`` runs on the Gateway event loop, both at shutdown and from
|
|
``POST /api/channels/{name}/restart``, so a blocking teardown call there stalls
|
|
every run, stream and channel the Gateway serves:
|
|
|
|
- Slack: ``SocketModeClient.close()`` joins the SDK's message-processor thread
|
|
and waits for in-flight listeners.
|
|
- Feishu / DingTalk: ``stop()`` asks the SDK thread's private event loop to
|
|
shut down, then joins that thread with a 5s timeout.
|
|
- Discord: ``stop()`` joins the client thread with a 10s timeout, which a
|
|
timed-out client close or a slow ``_run_client()`` drain can use up.
|
|
|
|
Each test drives the real ``stop()`` (and, for Slack, a real slack-sdk client).
|
|
If a join or close regresses back onto the event loop, the strict Blockbuster
|
|
gate raises ``BlockingError`` from the thread-join lock wait.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import asyncio
|
|
import threading
|
|
|
|
import pytest
|
|
|
|
from app.channels.base import ChannelStopTimeout
|
|
from app.channels.dingtalk import DingTalkChannel
|
|
from app.channels.discord import DiscordChannel
|
|
from app.channels.feishu import FeishuChannel
|
|
from app.channels.message_bus import MessageBus
|
|
from app.channels.slack import SlackChannel
|
|
|
|
pytestmark = pytest.mark.asyncio
|
|
|
|
|
|
async def test_slack_stop_closes_real_socket_mode_client_off_loop() -> None:
|
|
from slack_sdk.socket_mode import SocketModeClient
|
|
|
|
# Never connected: construction alone starts the SDK's worker threads,
|
|
# which is what close() has to join.
|
|
socket_client = SocketModeClient(app_token="xapp-test")
|
|
channel = SlackChannel(bus=MessageBus(), config={})
|
|
channel._socket_client = socket_client
|
|
channel._running = True
|
|
|
|
await channel.stop()
|
|
|
|
assert socket_client.closed is True
|
|
assert channel._socket_client is None
|
|
|
|
|
|
async def test_slack_stop_keeps_connect_cleanup_off_loop(monkeypatch) -> None:
|
|
started, release = threading.Event(), threading.Event()
|
|
loop_thread = threading.get_ident()
|
|
close_threads = []
|
|
|
|
class SocketClient:
|
|
def connect(self):
|
|
started.set()
|
|
release.wait(5)
|
|
|
|
def close(self):
|
|
close_threads.append(threading.get_ident())
|
|
|
|
monkeypatch.setattr("app.channels.slack.SLACK_STOP_TIMEOUT_SECONDS", 0.02)
|
|
channel = SlackChannel(MessageBus(), {})
|
|
client = SocketClient()
|
|
channel._socket_client = client
|
|
channel._running = True
|
|
future = asyncio.get_running_loop().run_in_executor(None, channel._connect_socket_mode, client)
|
|
channel._socket_connect_future = future
|
|
try:
|
|
assert await asyncio.to_thread(started.wait, 2)
|
|
with pytest.raises(ChannelStopTimeout):
|
|
await channel.stop()
|
|
assert not future.cancelled()
|
|
assert close_threads == []
|
|
release.set()
|
|
await asyncio.wait_for(asyncio.shield(channel._socket_close_task), 2)
|
|
await channel.stop()
|
|
assert len(close_threads) == 1
|
|
assert close_threads[0] != loop_thread
|
|
assert channel._socket_client is None
|
|
finally:
|
|
release.set()
|
|
await asyncio.wait_for(asyncio.shield(future), 5)
|
|
monkeypatch.setattr("app.channels.slack.SLACK_STOP_TIMEOUT_SECONDS", 3)
|
|
await channel.stop()
|
|
|
|
|
|
@pytest.mark.parametrize("channel_cls", [FeishuChannel, DingTalkChannel, DiscordChannel])
|
|
async def test_stop_joins_sdk_thread_off_loop(channel_cls) -> None:
|
|
release = threading.Event()
|
|
sdk_thread = threading.Thread(target=release.wait, daemon=True)
|
|
sdk_thread.start()
|
|
channel = channel_cls(MessageBus(), {})
|
|
channel._thread = sdk_thread
|
|
channel._running = True
|
|
release.set()
|
|
|
|
await channel.stop()
|
|
|
|
assert not sdk_thread.is_alive()
|
|
assert channel._thread is None
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
("channel_cls", "loop_attr"),
|
|
[(FeishuChannel, "_ws_loop"), (DingTalkChannel, "_stream_loop")],
|
|
)
|
|
async def test_stop_ends_live_sdk_loop_off_loop(channel_cls, loop_attr) -> None:
|
|
sdk_loop = asyncio.new_event_loop()
|
|
ready = threading.Event()
|
|
|
|
def run_sdk_loop() -> None:
|
|
asyncio.set_event_loop(sdk_loop)
|
|
sdk_loop.call_soon(ready.set)
|
|
try:
|
|
sdk_loop.run_forever()
|
|
finally:
|
|
sdk_loop.close()
|
|
|
|
sdk_thread = threading.Thread(target=run_sdk_loop, daemon=True)
|
|
sdk_thread.start()
|
|
assert await asyncio.to_thread(ready.wait, 5)
|
|
channel = channel_cls(MessageBus(), {})
|
|
channel._thread = sdk_thread
|
|
channel._running = True
|
|
setattr(channel, loop_attr, sdk_loop)
|
|
|
|
await channel.stop()
|
|
|
|
assert not sdk_thread.is_alive()
|
|
assert sdk_loop.is_closed()
|
|
assert channel._thread is None
|
|
assert getattr(channel, loop_attr) is None
|