[PyFDB] GIL dead-lock on threading usage - #292
Draft
tbkr wants to merge 2 commits into
Draft
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #292 +/- ##
========================================
Coverage 71.47% 71.47%
========================================
Files 372 372
Lines 23510 23510
Branches 2464 2464
========================================
Hits 16803 16803
Misses 6707 6707 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
tbkr
force-pushed
the
feature/pyfdb/fdb-685-GIL-threading
branch
2 times, most recently
from
June 30, 2026 13:00
13e2292 to
3eae28b
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a Python GIL-related deadlock in PyFDB by ensuring the pybind11 bindings release the GIL around potentially blocking C++ FDB calls, and adds new threading-focused tests (plus a skipped remote/proxy E2E test) to prevent regressions.
Changes:
- Release the GIL in pybind11 bindings for blocking FDB/DataHandle operations and iterators to avoid deadlocks when other Python threads must run (e.g., port-forwarding).
- Add new PyFDB integration threading tests and a (currently skipped) remote/proxy E2E test + supporting E2E fixtures.
- Minor formatting/tooling/CI updates (ruff config, artifact action version bumps, small Python formatting change).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/pyfdb/integration/test_threading.py | New integration tests validating multi-threaded list/retrieve/archive behavior and guarding against GIL deadlocks. |
| tests/pyfdb/e2e/remote_threading/test_pyfdb_remote_fdb_threading.py | New (skipped) E2E test simulating a Python proxy/port-forwarder to reproduce the deadlock scenario end-to-end. |
| tests/pyfdb/e2e/conftest.py | New E2E fixtures to spin up store+catalogue servers and generate configs for remote testing. |
| tests/pyfdb/CMakeLists.txt | Registers the new integration and E2E tests with CTest. |
| src/pyfdb/pyfdb.py | Small formatting change when building raw control identifiers. |
| src/pyfdb_bindings/bindings.cc | Core fix: apply GIL release strategy in pybind11 bindings for blocking FDB/DataHandle calls and iterators. |
| pyproject.toml | Adds ruff configuration (line length/formatting). |
| .github/workflows/z3fdb.yml | Updates artifact upload/download actions to newer major versions. |
| .github/workflows/pyfdb.yml | Updates artifact upload/download actions to newer major versions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jharwood0
reviewed
Jul 2, 2026
tbkr
force-pushed
the
feature/pyfdb/fdb-685-GIL-threading
branch
2 times, most recently
from
July 7, 2026 15:54
30425aa to
cf694d3
Compare
jharwood0
reviewed
Jul 8, 2026
Contributor
There was a problem hiding this comment.
Just to check, was this intentionally removed?
Contributor
Author
There was a problem hiding this comment.
For some reason I can't see the line the comment is referring to.
jharwood0
approved these changes
Jul 8, 2026
tbkr
force-pushed
the
feature/pyfdb/fdb-685-GIL-threading
branch
8 times, most recently
from
July 15, 2026 09:35
7448a4b to
05d0e3c
Compare
tbkr
marked this pull request as draft
July 15, 2026 12:36
tbkr
force-pushed
the
feature/pyfdb/fdb-685-GIL-threading
branch
5 times, most recently
from
July 28, 2026 11:38
315adf5 to
ef4b6cf
Compare
GIL released on functions which do heavy C++ work, are long-running or destruct C++ objects. feature: Add threading tests Local threading tests for pyfdb. The remote fdb threading test is skipped currently till #291 is merged. chore: Update workflow of github cicd fix: Adjust GIL use in iterators Before accessing the py::stop_iteration, we need to aquire the GIL chore: Add pyproject.toml for ruff format bugFix: Addressing issue with read item size The wrong item size was handed to the read call. This never failed because we are internally using bytearrays for reading (having size=1). chore: Typo fix chore: Minor updates to the threading tests feat: Add tests for threaded partial reading block scenario feat: test_lifetime.py for datahandle life time test fix: Fix fdb-server on PATH in CI docs: Added docs section for multi threading
Compute the unique temp schema path BEFORE taking local_mutex. PathName::unique() acquires eckit::LocalPathName's own StaticMutex; holding TocHandler::local_mutex while doing so nests two StaticMutexes, which deadlocks against eckit's pthread_atfork prepare handler (get_locks() locks every registered static mutex in address order). unique() only generates a name (mutex-guarded static counter) and mkdir's the parent dir — it does not create the temp file — so it is safe here.
tbkr
force-pushed
the
feature/pyfdb/fdb-685-GIL-threading
branch
from
July 28, 2026 11:58
ef4b6cf to
4b4a020
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The original issue was risen in #289. This PR implements a fix for threading deadlock in the pyfdb and add local and remote tests.
The remote tests are skipped till #291 is merged.
Contributor Declaration
By opening this pull request, I affirm the following:
🌈🌦️📖🚧 Documentation FDB 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/fdb/pull-requests/PR-292