Skip to content

Commit e4e3508

Browse files
committed
fix(python): release GIL before blocking on tokio runtime
Wrap all rt.block_on() calls in py.detach() to release the GIL before blocking. This prevents deadlock when tool callbacks call Python::attach() to reacquire the GIL in multi-threaded scenarios. Fixed in: PyBash::execute_sync, PyBash::reset, BashTool::execute_sync, BashTool::reset, ScriptedTool::execute_sync. https://claude.ai/code/session_01WZjYqxm5xMPAEe7FSHJkDy
1 parent 90bbd7a commit e4e3508

File tree

2 files changed

+187
-52
lines changed

2 files changed

+187
-52
lines changed

crates/bashkit-python/src/lib.rs

Lines changed: 67 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -233,41 +233,47 @@ impl PyBash {
233233
}
234234

235235
/// Execute commands synchronously (blocking).
236-
fn execute_sync(&self, commands: String) -> PyResult<ExecResult> {
236+
/// Releases GIL before blocking on tokio to prevent deadlock with callbacks.
237+
fn execute_sync(&self, py: Python<'_>, commands: String) -> PyResult<ExecResult> {
237238
let inner = self.inner.clone();
238239
let rt = tokio::runtime::Runtime::new()
239240
.map_err(|e| PyRuntimeError::new_err(format!("Failed to create runtime: {}", e)))?;
240241

241-
rt.block_on(async move {
242-
let mut bash = inner.lock().await;
243-
match bash.exec(&commands).await {
244-
Ok(result) => Ok(ExecResult {
245-
stdout: result.stdout,
246-
stderr: result.stderr,
247-
exit_code: result.exit_code,
248-
error: None,
249-
}),
250-
Err(e) => Ok(ExecResult {
251-
stdout: String::new(),
252-
stderr: String::new(),
253-
exit_code: 1,
254-
error: Some(e.to_string()),
255-
}),
256-
}
242+
py.detach(|| {
243+
rt.block_on(async move {
244+
let mut bash = inner.lock().await;
245+
match bash.exec(&commands).await {
246+
Ok(result) => Ok(ExecResult {
247+
stdout: result.stdout,
248+
stderr: result.stderr,
249+
exit_code: result.exit_code,
250+
error: None,
251+
}),
252+
Err(e) => Ok(ExecResult {
253+
stdout: String::new(),
254+
stderr: String::new(),
255+
exit_code: 1,
256+
error: Some(e.to_string()),
257+
}),
258+
}
259+
})
257260
})
258261
}
259262

260263
/// Reset interpreter to fresh state.
261-
fn reset(&self) -> PyResult<()> {
264+
/// Releases GIL before blocking on tokio to prevent deadlock.
265+
fn reset(&self, py: Python<'_>) -> PyResult<()> {
262266
let inner = self.inner.clone();
263267
let rt = tokio::runtime::Runtime::new()
264268
.map_err(|e| PyRuntimeError::new_err(format!("Failed to create runtime: {}", e)))?;
265269

266-
rt.block_on(async move {
267-
let mut bash = inner.lock().await;
268-
let builder = Bash::builder();
269-
*bash = builder.build();
270-
Ok(())
270+
py.detach(|| {
271+
rt.block_on(async move {
272+
let mut bash = inner.lock().await;
273+
let builder = Bash::builder();
274+
*bash = builder.build();
275+
Ok(())
276+
})
271277
})
272278
}
273279

@@ -378,40 +384,46 @@ impl BashTool {
378384
})
379385
}
380386

381-
fn execute_sync(&self, commands: String) -> PyResult<ExecResult> {
387+
/// Releases GIL before blocking on tokio to prevent deadlock with callbacks.
388+
fn execute_sync(&self, py: Python<'_>, commands: String) -> PyResult<ExecResult> {
382389
let inner = self.inner.clone();
383390
let rt = tokio::runtime::Runtime::new()
384391
.map_err(|e| PyRuntimeError::new_err(format!("Failed to create runtime: {}", e)))?;
385392

386-
rt.block_on(async move {
387-
let mut bash = inner.lock().await;
388-
match bash.exec(&commands).await {
389-
Ok(result) => Ok(ExecResult {
390-
stdout: result.stdout,
391-
stderr: result.stderr,
392-
exit_code: result.exit_code,
393-
error: None,
394-
}),
395-
Err(e) => Ok(ExecResult {
396-
stdout: String::new(),
397-
stderr: String::new(),
398-
exit_code: 1,
399-
error: Some(e.to_string()),
400-
}),
401-
}
393+
py.detach(|| {
394+
rt.block_on(async move {
395+
let mut bash = inner.lock().await;
396+
match bash.exec(&commands).await {
397+
Ok(result) => Ok(ExecResult {
398+
stdout: result.stdout,
399+
stderr: result.stderr,
400+
exit_code: result.exit_code,
401+
error: None,
402+
}),
403+
Err(e) => Ok(ExecResult {
404+
stdout: String::new(),
405+
stderr: String::new(),
406+
exit_code: 1,
407+
error: Some(e.to_string()),
408+
}),
409+
}
410+
})
402411
})
403412
}
404413

405-
fn reset(&self) -> PyResult<()> {
414+
/// Releases GIL before blocking on tokio to prevent deadlock.
415+
fn reset(&self, py: Python<'_>) -> PyResult<()> {
406416
let inner = self.inner.clone();
407417
let rt = tokio::runtime::Runtime::new()
408418
.map_err(|e| PyRuntimeError::new_err(format!("Failed to create runtime: {}", e)))?;
409419

410-
rt.block_on(async move {
411-
let mut bash = inner.lock().await;
412-
let builder = Bash::builder();
413-
*bash = builder.build();
414-
Ok(())
420+
py.detach(|| {
421+
rt.block_on(async move {
422+
let mut bash = inner.lock().await;
423+
let builder = Bash::builder();
424+
*bash = builder.build();
425+
Ok(())
426+
})
415427
})
416428
}
417429

@@ -652,17 +664,20 @@ impl ScriptedTool {
652664
}
653665

654666
/// Execute a bash script synchronously (blocking).
655-
fn execute_sync(&self, commands: String) -> PyResult<ExecResult> {
667+
/// Releases GIL before blocking on tokio to prevent deadlock with callbacks.
668+
fn execute_sync(&self, py: Python<'_>, commands: String) -> PyResult<ExecResult> {
656669
let mut tool = self.build_rust_tool();
657670
let rt = tokio::runtime::Runtime::new()
658671
.map_err(|e| PyRuntimeError::new_err(format!("Failed to create runtime: {}", e)))?;
659672

660-
let resp = rt.block_on(async move {
661-
tool.execute(ToolRequest {
662-
commands,
663-
timeout_ms: None,
673+
let resp = py.detach(|| {
674+
rt.block_on(async move {
675+
tool.execute(ToolRequest {
676+
commands,
677+
timeout_ms: None,
678+
})
679+
.await
664680
})
665-
.await
666681
});
667682
Ok(ExecResult {
668683
stdout: resp.stdout,

crates/bashkit-python/tests/test_bashkit.py

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -728,3 +728,123 @@ async def test_async_multiple_tools():
728728
assert r.exit_code == 0
729729
assert "A" in r.stdout
730730
assert "B" in r.stdout
731+
732+
733+
# ===========================================================================
734+
# GIL deadlock prevention tests
735+
# ===========================================================================
736+
737+
738+
def test_execute_sync_releases_gil_for_callback():
739+
"""execute_sync must release GIL before blocking on tokio runtime.
740+
741+
Without py.allow_threads(), a tool callback calling Python::attach()
742+
inside rt.block_on() can deadlock when another thread holds the GIL.
743+
This test validates the fix by running execute_sync with a callback
744+
from a background thread while the main thread also holds the GIL.
745+
"""
746+
import threading
747+
import time
748+
749+
tool = ScriptedTool("api")
750+
751+
def slow_callback(params, stdin=None):
752+
"""Callback that briefly sleeps, requiring GIL reacquisition."""
753+
time.sleep(0.01)
754+
return f"ok-{params.get('id', 0)}\n"
755+
756+
tool.add_tool(
757+
"slow",
758+
"Slow command",
759+
callback=slow_callback,
760+
schema={"type": "object", "properties": {"id": {"type": "integer"}}},
761+
)
762+
763+
results = [None, None]
764+
errors = [None, None]
765+
766+
def run_in_thread(idx):
767+
try:
768+
r = tool.execute_sync(f"slow --id {idx}")
769+
results[idx] = r
770+
except Exception as e:
771+
errors[idx] = e
772+
773+
t0 = threading.Thread(target=run_in_thread, args=(0,))
774+
t1 = threading.Thread(target=run_in_thread, args=(1,))
775+
776+
t0.start()
777+
t1.start()
778+
779+
# Timeout guards against deadlock — if GIL isn't released, threads block forever
780+
t0.join(timeout=10)
781+
t1.join(timeout=10)
782+
783+
assert not t0.is_alive(), "Thread 0 deadlocked (GIL not released in execute_sync)"
784+
assert not t1.is_alive(), "Thread 1 deadlocked (GIL not released in execute_sync)"
785+
assert errors[0] is None, f"Thread 0 error: {errors[0]}"
786+
assert errors[1] is None, f"Thread 1 error: {errors[1]}"
787+
assert results[0].exit_code == 0
788+
assert results[1].exit_code == 0
789+
assert "ok-0" in results[0].stdout
790+
assert "ok-1" in results[1].stdout
791+
792+
793+
def test_bash_execute_sync_releases_gil():
794+
"""Bash.execute_sync must release GIL before blocking on tokio runtime."""
795+
import threading
796+
797+
bash = Bash()
798+
results = [None, None]
799+
errors = [None, None]
800+
801+
def run_in_thread(idx):
802+
try:
803+
r = bash.execute_sync(f"echo thread-{idx}")
804+
results[idx] = r
805+
except Exception as e:
806+
errors[idx] = e
807+
808+
t0 = threading.Thread(target=run_in_thread, args=(0,))
809+
t1 = threading.Thread(target=run_in_thread, args=(1,))
810+
811+
t0.start()
812+
t1.start()
813+
814+
t0.join(timeout=10)
815+
t1.join(timeout=10)
816+
817+
assert not t0.is_alive(), "Thread 0 deadlocked (GIL not released)"
818+
assert not t1.is_alive(), "Thread 1 deadlocked (GIL not released)"
819+
assert errors[0] is None, f"Thread 0 error: {errors[0]}"
820+
assert errors[1] is None, f"Thread 1 error: {errors[1]}"
821+
822+
823+
def test_bashtool_execute_sync_releases_gil():
824+
"""BashTool.execute_sync must release GIL before blocking on tokio runtime."""
825+
import threading
826+
827+
tool = BashTool()
828+
results = [None, None]
829+
errors = [None, None]
830+
831+
def run_in_thread(idx):
832+
try:
833+
r = tool.execute_sync(f"echo thread-{idx}")
834+
results[idx] = r
835+
except Exception as e:
836+
errors[idx] = e
837+
838+
t0 = threading.Thread(target=run_in_thread, args=(0,))
839+
t1 = threading.Thread(target=run_in_thread, args=(1,))
840+
841+
t0.start()
842+
t1.start()
843+
844+
t0.join(timeout=10)
845+
t1.join(timeout=10)
846+
847+
assert not t0.is_alive(), "Thread 0 deadlocked (GIL not released)"
848+
assert not t1.is_alive(), "Thread 1 deadlocked (GIL not released)"
849+
assert errors[0] is None, f"Thread 0 error: {errors[0]}"
850+
assert errors[1] is None, f"Thread 1 error: {errors[1]}"

0 commit comments

Comments
 (0)