fix(storage): retain exact resources across interruption - #622
Conversation
fishmingyu
added
the
full-ci
e9fe251 to
0153d61
Compare
Publish POSIX descriptor and Windows HANDLE records before native acquisition. Route failure cleanup through exact-record owners so interrupted cleanup cannot sweep unrelated resources, and carry the same ownership protocol through Windows tree scans while preserving the first failure. This leaves the documented native-return/first-STORE and sentinel-generation limits unchanged. Verified with Python 3.10, 3.12, and 3.14 atomic tests, Python 3.14 direct-consumer storage tests (512 passed, 10 skipped), and the local unit tier (5152 passed, 71 skipped, 190 deselected).
0153d61 to
cc74b6c
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc74b6c186
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| except BaseException as error: # noqa: B036 - preserve exact local primary | ||
| context_error = error | ||
| failures.primary_error = error | ||
| failures.protect_pending_owners() |
There was a problem hiding this comment.
When the context body has already raised and a cancellation such as KeyboardInterrupt lands while this newly added call enters or executes, it escapes the except block before the bare raise. Although the finally retains context_error internally, locally_unwinding is then true, so the helper does not explicitly re-raise that original error and the cancellation becomes the outward exception instead. This is reproducible by making the first protect_pending_owners() invocation raise: a body ValueError is replaced by that interruption. Move this protection into the guarded finalizer path or otherwise ensure the stored body error remains primary.
Useful? React with 👍 / 👎.
| if record.identity is not None and observed_identity != record.identity: | ||
| record.descriptor = -1 | ||
| return True |
There was a problem hiding this comment.
When an owned descriptor has been closed and its numeric value reused for a different object before exact cleanup, this predicate clears the record and reports completion. Since protect_pending_owners() and the ordered runner invoke closed before the cleanup action, close_record() is then skipped and its publication descriptor ownership changed error is never retained; the mirrored Windows predicate has the same behavior. The previous cleanup path surfaced this mismatch as a diagnostic while still refusing to close the replacement, so leave identity mismatches incomplete long enough for _close_record() to report them rather than silently treating them as successful cleanup.
Useful? React with 👍 / 👎.
1 participant
Summary
Harden the shared atomic-directory ownership path so native resources remain reachable and exactly cleanable when interruption lands around acquisition or cleanup.
This low-level fix is intentionally separate from #619 because it changes the common POSIX and Windows publication foundation and should land before the workspace API feature.
Changes
Type of Change
Testing
Tests pass locally
Added new tests for the changes
Python 3.10, 3.12, and 3.14 atomic-directory tests: 256 passed, 10 skipped on each runtime
Python 3.14 direct-consumer storage suites: 512 passed, 10 skipped
local unit tier excluding the unavailable Docker sandbox: 5152 passed, 71 skipped, 190 deselected
Black, isort, flake8, py_compile, and git diff --check
Checklist