Closed tornaria closed 9 months ago
Rebased, to run the current tests
@malb @dimpase should this be merged? It seems to pass all tests. @tornaria Are you using these fixes in production already?
@malb @dimpase should this be merged? It seems to pass all tests. @tornaria Are you using these fixes in production already?
I'm only using #162 (see https://github.com/void-linux/void-packages/tree/master/srcpkgs/python3-cysignals/patches). This is the only issue that I know how to reproduce. This PR "fixes" potential issues of the same kind that I don't know how to reproduce.
Note that #162 is merged but never released. It would be nice to have a new release which fixes this issue and fully supports cython 3. The easy way to support cython 3 is to use legacy_implicit_noexcept
but a proper fix is to actually add all the noexcept
clauses so the code builds fine with cython 0.29 or cython 3.
rebased over the latest main, to see how it goes with CI
Follow up to #162.
The first commit gets rid of all usage of stdio in signal handlers (mostly, but not only, inside
ENABLE_DEBUG_CYSIGNALS
).The second commit replaces use of
gettimeofday()
byclock_gettime()
, the latter being safe according tosignal-safety(7)
. It's also monotonic, unlikegettimeofday()
.Other stuff that happens inside the signal handler:
cysigs_interrupt_handler()
callsPyErr_SetInterrupt()
(probably ok?).when
debug_level >= 2
the functiondo_raise_exception()
calls PyGILState_Ensure / PyErr_Occurred / PyGILState_Release, I wonder if this could deadlock for some reason; in any case I don't understand why the GIL is necessary if the return value of PyErr_Occurred() is not used, just printed.function
sigdie()
usesgetenv()
which is not safe, maybe the env variables should be read byinit_cysignals()
instead.function
print_enhanced_backtrace()
usesexecvp()
, this is probably ok although the "correct" way would be to read PATH on init and here useexecv()
with the full path tocysignals-CSI
already precomputed.the function
sig_raise_exception()
is in cython and calls lots of python stuff, I wonder if this should be called after the longjmp() instead (it probably makes no difference since I don't thinklongjmp()
will fix internal state of e.g. stdio).