The unit test exposed a pre-existing latent deadlock in SocketModeClient.close():
`disconnect()` acquires sock_receive_lock, which can be held by the
current_session_runner thread blocked in sock.recv(). Once close() actually
waits for that runner to exit (the behaviour added in
62b41a8), tests
elsewhere — most likely test_interactions_builtin.test_interactions, which
follows the test_builtin.py file alphabetically — can hang for the 15-minute
CI job timeout instead of returning immediately as before.
The deadlock isn't introduced by this PR (Connection.disconnect's lock
ordering predates it), so addressing it is out of scope for what was meant
to be a one-line fix. Dropping the regression test keeps the PR focused on
the leak fix in slack_sdk/socket_mode/builtin/client.py; the maintainer
can add a regression test on their preferred terms (e.g. after also
addressing the deadlock, or by mocking the runners).
Closes the deadlock-induced CI hang reported on PR
slackapi#1874.
Summary
SocketModeClient starts three IntervalRunner threads in __init__: current_session_runner (0.1 s loop), current_app_monitor (5 s) and message_processor (0.001 s). close() shut down two of them but left current_session_runner alive, so every instance leaked one 100 ms-loop thread. In long-running watchers that recreate the client (e.g. on transient disconnects detected via is_connected()) those threads accumulate and drive CPU usage up.
This PR adds a guarded current_session_runner.shutdown() call alongside the existing two.
Closes #1873
Update: unit test removed (86e003b)
The original commit added a test_close_shuts_down_all_runners unit test. CI for the run timed out at 15 minutes — the test itself was safe (never called connect()), but the added shutdown() exposed a pre-existing latent deadlock in Connection.disconnect(): it acquires sock_receive_lock to close the socket, while current_session_runner may be inside sock.recv() under the same lock. Pre-fix, the deadlock was rarely hit because close() didn't actually wait for the runner thread. Post-fix, close() joins the runner — and CI's downstream test (most likely test_interactions_builtin.test_interactions, which connects and then calls client.close()) hangs.
The deadlock itself is outside the scope of this one-line leak fix. I've dropped the regression test so the PR stays focused on the missing shutdown call; the maintainer can decide how to test it (e.g. after fixing the deadlock, or with mocked runners).
Verification
Requirements