-
-
Notifications
You must be signed in to change notification settings - Fork 3k
[mypyc] Fix object finalization #19749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
cf3dd49
to
5b92c8b
Compare
emitter.emit_line("if (PyErr_Occurred() != NULL) {") | ||
emitter.emit_line("PyErr_WriteUnraisable((PyObject *)self);") | ||
emitter.emit_line("}") | ||
emitter.emit_line("PyErr_Restore(type, value, traceback);") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would move this code to the done
label below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to work unfortunately. If I move it, other tests start to fail. E.g. mypyc/test/test_run.py::TestRun::run-exceptions.test::testTryExcept
with:
Traceback (most recent call last): (diff)
File "testutil.py", line 46, in assertRaises (diff)
yield (diff)
File "driver.py", line 42, in <module> (diff)
list(iter_exception()) (diff)
~~~~^^^^^^^^^^^^^^^^^^ (diff)
SystemError: error return without exception set (diff)
(diff)
During handling of the above exception, another exception occurred: (diff)
(diff)
Traceback (most recent call last): (diff)
File "driver.py", line 41, in <module> (diff)
with assertRaises(IndexError): (diff)
~~~~~~~~~~~~^^^^^^^^^^^^ (diff)
File "/Users/marc/Develop/01_temp/cpython/Lib/contextlib.py", line 162, in __exit__ (diff)
self.gen.throw(value) (diff)
~~~~~~~~~~~~~~^^^^^^^ (diff)
File "testutil.py", line 48, in assertRaises (diff)
assert type(e) is typ, f"{e!r} is not a {typ.__name__}" (diff)
^^^^^^^^^^^^^^ (diff)
AssertionError: SystemError('error return without exception set') is not a IndexError (diff)
So I think PyErr_Restore
needs to happen earlier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, that's not making sense to me. Is the done
label not getting hit in all cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is, but some cleanup functions from mypyc seem to require that the exception is set properly.
Just the goto done;
call is never used AFAICT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, that's a little messy. As long as they don't overwrite the exception, I guess it's fine.
Hmm, not quite sure why the exit code is |
That's the segfault exit code. |
I can remove the |
025423f
to
f62a220
Compare
Got it fixed. If an error occurred and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix! I verified that this fixes the segfault on macOS and Python 3.13.
Fixes mypyc/mypyc#1127