python: let KeyboardInterrupt and SystemExit out of a callback (ibx#270) - #390
Open
userFRM wants to merge 1 commit into
Open
python: let KeyboardInterrupt and SystemExit out of a callback (ibx#270)#390userFRM wants to merge 1 commit into
userFRM wants to merge 1 commit into
Conversation
call_wrapper! in src/python/compat/client/dispatch.rs wrapped every Python wrapper call and, on error, logged the exception then discarded it unconditionally via e.restore(py) followed by PyErr_Clear() — a pair that nets out to the same as dropping e, since restoring an exception immediately before clearing it leaves nothing set. KeyboardInterrupt and SystemExit derive from BaseException rather than Exception precisely so that blanket handlers do not eat them, and this handler ate them along with everything else. Two consequences followed. CPython delivers Ctrl-C by raising KeyboardInterrupt in whatever frame is executing when the signal arrives; if that frame was a user callback invoked through call_wrapper!, the exception surfaced as the call_method error and was discarded before run()'s py.check_signals() ever ran, so the interrupt was only caught when it landed in the wait window between dispatch iterations. And a callback raising SystemExit — the mechanism ibapi's own dispatch loop honors so error() or order_status() can stop the client — was logged and swallowed instead of terminating run(). The macro now checks e.is_instance_of::<pyo3::exceptions::PyException>(py) before deciding what to do with the error. Anything deriving from Exception is logged and dropped, same as before. Anything that is a BaseException but not an Exception — KeyboardInterrupt, SystemExit, and anything else in that category — is returned instead. Every call_wrapper! invocation in dispatch_once sits directly in the function body rather than inside a closure, so that return unwinds through dispatch_once's PyResult<()>, through run()'s ? on the dispatch_once call, and back to the Python caller as the original exception, letting run() stop the same way ibapi's dispatch loop does. The restore/PyErr_Clear pair is gone; letting e drop at the end of the swallow branch already discards it. cargo check --offline --lib --features python is clean and the full gate suite passes (GATES CLEAN), with the two config::expiry_tests tzdata failures that are expected in this environment. cargo test --lib --features python does not link here per ibx#381, and no Python-level test was run either — there is no built extension module in this tree and building one would hit the same linkage gap. The propagation path was checked by reading every call_wrapper! call site to confirm none sit inside a closure, and against pyo3 0.29's exception hierarchy, where PyKeyboardInterrupt and PySystemExit both derive from PyBaseException rather than PyException. Closes deepentropy#270.
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.
Problem
Every Python callback was invoked through a macro that caught whatever it raised, logged it, and carried on. That is right for an ordinary exception — one bad callback should not kill the dispatch loop — but it applied to
KeyboardInterruptandSystemExittoo.So Ctrl-C pressed while a callback was running was logged and discarded, and a callback raising
SystemExitcould not stoprun().What this changes
An exception that derives from
BaseExceptionbut notExceptionis re-raised instead of logged. That is exactly the set Python reserves for control flow —KeyboardInterrupt,SystemExit,GeneratorExit— and it is the same line ibapi draws. Ordinary exceptions are still caught and logged, unchanged.Every call site sits directly in
dispatch_once's body, so the re-raise unwinds through itsPyResult<()>, throughrun()'s?, and back to the Python caller as the original exception. Verified by walking the brace structure of the function rather than by grep: the function contains ten closure bodies, and no call site is inside one — areturnin a closure would have exited the closure and swallowed the interrupt anyway.The
restore-then-PyErr_Clearpair on the swallow path is removed. It nets out to dropping the error, which is what letting it fall out of scope does.Verification
cargo check --offline --lib --features pythonis clean, and the full gate passes.Not run:
cargo test --lib --features python, which does not link in this tree — a stale test blocks compilation and libpython is then missing (#381). The propagation path is therefore established by reading the call-site placement, the return-type plumbing, and pyo3's exception hierarchy rather than by executing it.Closes #270.