Repository navigation
Use recursive mutex to deal with GIL <-> internal lock deadlocks - #462
Merged
Merged
Conversation
2 tasks done
evertlammerts
force-pushed
the
fix_435_456
branch
from
May 19, 2026 09:12
3e71c0a to
74fecdb
Compare
kaka11chen
added a commit
to AstroVela/lance-duckdb
that referenced
this pull request
Nov 8, 2026
Windows drive paths such as `C:\...` and `D:/...` were parsed as non-file URI schemes in mutation workers. They consequently reopened the dataset through Lance's direct local writer, bypassing the object-store wrapper that tracks mutation files. UPDATE replacements and partial-DELETE files could be written and then rejected by transaction artifact validation. Use upstream `uri_to_url()` and OS-aware `to_file_path()` before selecting `file-object-store`. Resolve the C++ error-redaction policy before the mutation FFI call so path normalization cannot erase its Rust error. Keep artifact validation and diagnostic redaction in place. Regression coverage now includes local paths/file URIs with spaces, Unicode and literal percent escapes, plus the existing remote URI behavior. The existing Windows/macOS qualification script now exercises UPDATE, whole-fragment and partial DELETE, and full-table mutations in both local and Ray modes. The failed-UPDATE test requires the underlying nullability error rather than accepting only the generic transaction prefix. ### Validation - `cargo test --locked --offline --lib --features vane-distributed ffi::distributed_mutation::tests`: **5 passed**; compiled the changed Rust source. - C++ syntax compilation of `src/lance_distributed_write.cpp` with the released Vane 0.3.0 SDK headers and `LANCE_VANE_DISTRIBUTED`: **passed**. - `python -m pytest -q python/tests/test_vane_platform_release.py`: **23 passed**. - `uv run make format`, `cargo fmt --all --check`, Black, Python syntax checks and `git diff --check`: **passed**. - Updated qualification script: **local and Ray passed on Linux using the existing signed 0.3.0/0.3.0.1 wheels**. This checks the expanded script, not a newly linked provider. The Ray run used one node/two CPUs; inspection of 38 Ray log files found no mutation error or Python fatal. Windows verification of the rebuilt provider and the end-to-end error-detail regression remain pending. No TestPyPI/PyPI publication is part of this PR. ### Windows verification Use the matched Windows base/provider wheels built for this PR (the `vane-lance-full-windows` CI artifact), in a fresh CPython 3.12 x64 environment. The already published `0.3.0.1` wheel and the old test kit's pinned `install` step still select the old native code. From this PR checkout, after installing those CI wheels: ```powershell $env:VANE_EXPECTED_EXTENSION_TRUST_IDENTITY = "vane-ci-test-key" $env:VANE_DISTRIBUTED_NODE_COUNT = "1" $env:VANE_DISTRIBUTED_WORKER_SLOTS = "2" python scripts/verify_vane_platform.py --runner local --directory C:/vane-pr-test/local python scripts/verify_vane_platform.py --runner ray --directory C:/vane-pr-test/ray ``` Use fresh output directories. The script verifies signed loading, exact mutated rows and the empty result after full DELETE. Inspect actor stderr as well as command exit status. The separate GIL abort is tracked in [AstroVela/vane#1146](AstroVela/vane#1146), with the related upstream [duckdb-python#456](duckdb/duckdb-python#456) and [#462](duckdb/duckdb-python#462). That crash's exact native caller still needs a Windows stack; this mutation fix does not claim to fix it.
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.
Related to #435 and #456
#435 tries to use the same connection across multiple threads, which is not supported. Specifically, in this case the result is overwritten and destroyed concurrently, causing a segfault. This PR fixes the segfault by synchronising access to the result with a recursive mutex. But note that connections cannot be used across threads, no matter what.
#456: we sometimes need to destruct python-managed objects after releasing the GIL. This PR makes sure that we hold on to the GIL during the destruction of PyRelation and Connection::Close(). Note: I rely on CI to check whether the fix actually works here...