Skip to content

Commit 1de0a11

Browse files
WilliamBergaminClaude
andcommitted
fix(socket_mode): shut down current_session_runner in built-in close()
The built-in SocketModeClient starts three IntervalRunner daemon threads in __init__ (current_session_runner, current_app_monitor, message_processor) but close() only shut down two of them, leaking current_session_runner on every closed instance. Long-lived processes that open and close many clients accumulate one idle thread per closed instance (issue #1873). The sync sibling websocket_client.close() already shuts its current_session_runner down, and the async backends cancel all their futures. The leak was built-in only. Adding the shutdown alone is not enough: current_session_runner runs run_until_completion, whose loop exits only when the connection state is terminated. disconnect() does not set that flag, and shutdown() joins with no timeout, so the join would hang on a connected client. close() now sets current_session_state.terminated = True before disconnecting (mirroring connect()'s handling of a retired session), letting the loop return so the join completes. This is the CI hang the earlier #1874 attempt ran into. Adds a regression test on the connected path (connect then close) asserting all three runners are reaped; it hangs under pytest-timeout if terminated is not set, so it guards both the leak and the deadlock. Refs #1873. Supersedes #1874. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
1 parent c30a05d commit 1de0a11

2 files changed

Lines changed: 29 additions & 1 deletion

File tree

‎slack_sdk/socket_mode/builtin/client.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,10 +221,13 @@ def send_message(self, message: str) -> None:
221221
)
222222
raise e
223223

224-
def close(self):
224+
def close(self) -> None:
225225
self.closed = True
226226
self.auto_reconnect_enabled = False
227+
self.current_session_state.terminated = True
227228
self.disconnect()
229+
if self.current_session_runner.is_alive():
230+
self.current_session_runner.shutdown()
228231
if self.current_app_monitor.is_alive():
229232
self.current_app_monitor.shutdown()
230233
if self.message_processor.is_alive():

‎tests/slack_sdk/socket_mode/test_interactions_builtin.py‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,3 +145,28 @@ def test_send_message_while_disconnection(self):
145145
client.send_message("foo")
146146
finally:
147147
client.close()
148+
149+
def test_close_reaps_current_session_runner(self):
150+
# Regression for #1873: close() must shut down every interval-runner thread it started,
151+
# including current_session_runner, and must return (not hang) on the connected path.
152+
client = SocketModeClient(
153+
app_token="xapp-A111-222-xyz",
154+
web_client=self.web_client,
155+
auto_reconnect_enabled=False,
156+
trace_enabled=True,
157+
)
158+
try:
159+
client.wss_uri = "ws://0.0.0.0:3011/link"
160+
client.connect()
161+
self.assertTrue(client.is_connected())
162+
time.sleep(1) # let current_session_runner enter run_until_completion
163+
164+
client.close()
165+
166+
# The leaked thread from #1873 (RED -> GREEN driver):
167+
self.assertFalse(client.current_session_runner.is_alive())
168+
# Full-contract invariants: close() reaps all three runners:
169+
self.assertFalse(client.current_app_monitor.is_alive())
170+
self.assertFalse(client.message_processor.is_alive())
171+
finally:
172+
client.close()

0 commit comments

Comments
 (0)