Fix atexit unraisable exception message format#7399
Merged
youknowone merged 2 commits intoRustPython:mainfrom Mar 11, 2026
Merged
Fix atexit unraisable exception message format#7399youknowone merged 2 commits intoRustPython:mainfrom
youknowone merged 2 commits intoRustPython:mainfrom
Conversation
Match PyErr_FormatUnraisable behavior: use
"Exception ignored in atexit callback {func!r}" as err_msg
and pass None as object instead of the callback function.
Contributor
📝 WalkthroughWalkthroughAtexit callback storage and control flow were changed: registrations insert LIFO, unregistration searches backward with identity-aware removal, runner iterates stored entries forward and reports exceptions using a rendered func repr (passing None as source). PyGlobalState atexit storage now boxes each callback tuple. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant VM
participant AtexitList
participant Callback as "Callback Func"
Caller->>VM: register(callback, args)
VM->>AtexitList: insert_front(Box(callback,args))
Caller->>VM: unregister(callback)
VM->>AtexitList: search_backwards(&callback) -> identity_remove
Note over VM,AtexitList: At shutdown
VM->>AtexitList: take_list()
AtexitList-->>VM: list (preserving insertion order)
VM->>Callback: call(args)
alt exception
VM->>VM: try repr(callback)
VM->>VM: vm.run_unraisable(None, "Exception ignored in atexit callback <repr>")
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/vm/src/stdlib/atexit.rs`:
- Around line 41-49: The atexit cleanup currently passes Some("Exception ignored
in atexit callback") when func.repr(vm) fails, which diverges from CPython;
change the mapping so func.repr(vm) yields an Option<String> where Ok(repr) ->
Some(format!("Exception ignored in atexit callback {}", repr.as_wtf8())) and
Err(_) -> None, then call vm.run_unraisable(e, msg_option, vm.ctx.none());
update the intermediate variable (previously msg) to be an Option<String> to
match vm.run_unraisable's expected argument and preserve callback identity on
repr failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: fc792fce-8370-4d0b-aa18-21522eff31f9
⛔ Files ignored due to path filters (1)
Lib/test/_test_atexit.pyis excluded by!Lib/**
📒 Files selected for processing (1)
crates/vm/src/stdlib/atexit.rs
Release the lock during equality comparison in unregister so that __eq__ can safely call atexit.unregister or atexit._clear. Store callbacks in LIFO order (insert at front) and use identity-based search after comparison to handle list mutations, matching atexitmodule.c behavior. Also pass None as err_msg when func.repr() fails, matching CPython's PyErr_FormatUnraisable fallback.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/vm/src/stdlib/atexit.rs (1)
40-55: Minor theoretical race window in identity-based search.The
entry_ptris captured before releasing the lock (line 40), and used for pointer comparison after re-acquiring it (line 49). If the entry is removed and itsBoxdeallocated duringvm.bool_eq(), then a new allocation reuses the same address, there's a theoretical false-positive match.However, this is an acceptable trade-off given:
- The probability is extremely low (requires specific allocation patterns)
- The alternative (holding the lock during
__eq__) can cause deadlock when__eq__calls atexit functions- CPython's implementation has similar considerations around reentrant
__eq__🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/vm/src/stdlib/atexit.rs` around lines 40 - 55, The pointer-comparison can yield a false-positive if the boxed entry is deallocated and its address reused between releasing the lock and re-acquiring it; to fix, add a stable identity for entries instead of relying on raw pointer addresses: when pushing into vm.state.atexit_funcs include a unique id (e.g., a monotonically incrementing u64 or UUID) with each entry, then in the removal logic use that id for identity-based search rather than entry_ptr (update the producer that creates entries and the consumer code that searches/removes entries where vm.bool_eq is used); this avoids the theoretical allocation-reuse race while keeping the lock-release behavior around vm.bool_eq intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/vm/src/stdlib/atexit.rs`:
- Around line 40-55: The pointer-comparison can yield a false-positive if the
boxed entry is deallocated and its address reused between releasing the lock and
re-acquiring it; to fix, add a stable identity for entries instead of relying on
raw pointer addresses: when pushing into vm.state.atexit_funcs include a unique
id (e.g., a monotonically incrementing u64 or UUID) with each entry, then in the
removal logic use that id for identity-based search rather than entry_ptr
(update the producer that creates entries and the consumer code that
searches/removes entries where vm.bool_eq is used); this avoids the theoretical
allocation-reuse race while keeping the lock-release behavior around vm.bool_eq
intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 9b59e07b-9f98-4980-801e-20161fa6f2f9
⛔ Files ignored due to path filters (1)
Lib/test/_test_atexit.pyis excluded by!Lib/**
📒 Files selected for processing (2)
crates/vm/src/stdlib/atexit.rscrates/vm/src/vm/mod.rs
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
1 participant
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.
Match PyErr_FormatUnraisable behavior: use
"Exception ignored in atexit callback {func!r}" as err_msg and pass None as object instead of the callback function.
Summary by CodeRabbit
Behavior Changes
Bug Fixes