Skip to content

fix(ws): per-instance event loop instead of module-level global (fixes #119, fixes #133) - #153

Open
Xuxchloris wants to merge 1 commit into
larksuite:v2_mainfrom
Xuxchloris:fix/ws-per-instance-loop
Open

Xuxchloris wants to merge 1 commit into
larksuite:v2_mainfrom
Xuxchloris:fix/ws-per-instance-loop

Conversation

@Xuxchloris

Copy link
Copy Markdown

Fixes #119, fixes #133

Problem

lark_oapi/ws/client.py captured an event loop in a module-level global at import time:

try:
    loop = asyncio.get_event_loop()
except RuntimeError:
    loop = asyncio.new_event_loop()
    asyncio.set_event_loop(loop)

This broke two real scenarios:

  1. ws.Client: module-level loop variable prevents multiple instances (multi-bot race condition) #119 (multi-bot race) — all ws.Client instances share the single global loop. When several bots run in separate threads, they overwrite each other's loop and start() fails with RuntimeError: This event loop is already running.
  2. [Python lark-oapi 1.6.7] ws/client.py module-level event loop causes RuntimeError when first imported inside a running loop #133 (import inside a running loop) — the loop is captured at module import time. If the module is first imported inside asyncio.run(...), the global captures the already-running loop and the subsequent loop.run_until_complete(...) fails immediately.

Changes

  • lark_oapi/ws/client.py
    • Removed the module-level loop global entirely.
    • Each client owns a dedicated loop via _get_loop() (created lazily on first use, per instance) — distinct clients never share a loop.
    • The connect lock is created lazily (_get_lock()) from a coroutine running on the client's own loop, so the asyncio.Lock never binds to a foreign loop.
    • loop.create_task(...) inside async methods replaced with asyncio.get_running_loop().create_task(...).
  • lark_oapi/channel/tests/test_ws_client_loop.py (new): 4 regression tests — distinct clients get distinct loops (stable per instance), no module-level loop attribute remains, a client constructed inside asyncio.run gets a dedicated loop (not the running one), and the lock is created lazily.

Verification

  • python -m pytest lark_oapi/channel/tests — 666 passed; the single failure (test_upload_error_propagation.py::test_gather_buffer_missing_local_file_raises_upload_failed) is a pre-existing Windows-only path-escaping assertion unrelated to this change (CI runs on Linux).
  • Behavior for single-bot sync usage is unchanged: the first start() creates the client's loop and blocks on it exactly as before.

@bobbanga

bobbanga commented Oct 8, 2026

Copy link
Copy Markdown

Thanks for addressing #119 and #133. Could this change also update the SDK Channel shutdown path? In v1.7.3, Channel._stop_private_ws_client reads lark_oapi.ws.client.loop, so it still targets a shared module-level loop. Removing that global without changing this caller can leave Channel.stop() without the correct loop; host wrappers that reassign the global can point shutdown at another client's loop. Could shutdown use the specific WebSocket client's loop with no global fallback, and add a two-client regression asserting that stopping client A leaves client B running? An offline synthetic-socket reproduction is available if useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants