Skip to content

fix(python): avoid interpreter shutdown panic in BackgroundExecutor#6023

Merged
Xuanwo merged 1 commit intomainfrom
xuanwo/take-blobs-lifecycle-investigation
Feb 26, 2026
Merged

fix(python): avoid interpreter shutdown panic in BackgroundExecutor#6023
Xuanwo merged 1 commit intomainfrom
xuanwo/take-blobs-lifecycle-investigation

Conversation

@Xuanwo
Copy link
Copy Markdown
Collaborator

@Xuanwo Xuanwo commented Feb 26, 2026

Summary

  • harden BackgroundExecutor against Python interpreter finalization by replacing unconditional Python::attach(...) calls with Python::try_attach(...) in spawn, spawn_background, block_on, and signal polling loops
  • keep behavior the same when Python is attachable, and safely degrade to Ok(()) signal checks when Python is finalizing/unavailable
  • add a regression test that runs take_blobs + close in a subprocess and asserts no interpreter_lifecycle panic appears on process shutdown

Why

take_blobs could trigger a panic during interpreter shutdown because signal checks in the executor always called Python::attach(...), which panics when Python is finalizing.
This fix addresses the root cause globally in the executor instead of adding blob-specific special handling.

Testing

  • cargo check --manifest-path python/Cargo.toml --tests
  • cd python && uv sync --extra tests
  • cd python && uv run python -m pytest -q python/tests/test_blob.py

@github-actions github-actions Bot added bug Something isn't working python labels Feb 26, 2026
@github-actions
Copy link
Copy Markdown
Contributor

PR Review: fix(python): avoid interpreter shutdown panic in BackgroundExecutor

Overall: LGTM

This is a clean fix for the interpreter shutdown panic. Replacing Python::attach() with Python::try_attach() is the correct approach to gracefully handle the Python finalization state.

Reviewed aspects:

  • Code quality: The pattern used (let mut task = Some(task); ... task.take()) is the idiomatic workaround for the borrow checker with FnOnce closures. Changes are consistent across all four call sites.
  • Correctness: Falling back to Ok(()) for signal checks during shutdown is safe—we can't observe KeyboardInterrupt when Python is finalizing anyway.
  • Test coverage: The subprocess-based test is the right approach for testing interpreter shutdown behavior.

No P0/P1 issues found.

@Xuanwo Xuanwo merged commit c34fb82 into main Feb 26, 2026
14 checks passed
@Xuanwo Xuanwo deleted the xuanwo/take-blobs-lifecycle-investigation branch February 26, 2026 13:28
Xuanwo added a commit to wjones127/lance that referenced this pull request Feb 26, 2026
…ance-format#6023)

## Summary
- harden `BackgroundExecutor` against Python interpreter finalization by
replacing unconditional `Python::attach(...)` calls with
`Python::try_attach(...)` in `spawn`, `spawn_background`, `block_on`,
and signal polling loops
- keep behavior the same when Python is attachable, and safely degrade
to `Ok(())` signal checks when Python is finalizing/unavailable
- add a regression test that runs `take_blobs + close` in a subprocess
and asserts no `interpreter_lifecycle` panic appears on process shutdown

## Why
`take_blobs` could trigger a panic during interpreter shutdown because
signal checks in the executor always called `Python::attach(...)`, which
panics when Python is finalizing.
This fix addresses the root cause globally in the executor instead of
adding blob-specific special handling.

## Testing
- `cargo check --manifest-path python/Cargo.toml --tests`
- `cd python && uv sync --extra tests`
- `cd python && uv run python -m pytest -q python/tests/test_blob.py`
wjones127 pushed a commit that referenced this pull request Feb 26, 2026
…6023)

## Summary
- harden `BackgroundExecutor` against Python interpreter finalization by
replacing unconditional `Python::attach(...)` calls with
`Python::try_attach(...)` in `spawn`, `spawn_background`, `block_on`,
and signal polling loops
- keep behavior the same when Python is attachable, and safely degrade
to `Ok(())` signal checks when Python is finalizing/unavailable
- add a regression test that runs `take_blobs + close` in a subprocess
and asserts no `interpreter_lifecycle` panic appears on process shutdown

## Why
`take_blobs` could trigger a panic during interpreter shutdown because
signal checks in the executor always called `Python::attach(...)`, which
panics when Python is finalizing.
This fix addresses the root cause globally in the executor instead of
adding blob-specific special handling.

## Testing
- `cargo check --manifest-path python/Cargo.toml --tests`
- `cd python && uv sync --extra tests`
- `cd python && uv run python -m pytest -q python/tests/test_blob.py`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants