Skip to content

Commit 011d718

Browse files
author
Release Manager
committed
gh-40727: Explicitly check signum in GAP error handler Follow-up to #40613. Instead of checking for the string `user interrupt` (which might change between GAP versions, or if there's some unforeseen way the string might be sneaked in), we store the signum from the signal handler then check it in the GAP error handler. Also optionally use `AlarmInterrupt` instead of `KeyboardInterrupt` if cysignals is available. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] The title is concise and informative. - [ ] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - #12345: short description why this is a dependency --> <!-- - #34567: ... --> URL: #40727 Reported by: user202729 Reviewer(s): Michael Orlitzky, user202729
2 parents 23dd178 + 4010185 commit 011d718

File tree

2 files changed

+37
-16
lines changed

2 files changed

+37
-16
lines changed

src/sage/doctest/util.py

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -883,16 +883,9 @@ def ensure_interruptible_after(seconds: float, max_wait_after_interrupt: float =
883883

884884
try:
885885
yield data
886-
except KeyboardInterrupt as e:
887-
# AlarmInterrupt is a subclass of KeyboardInterrupt, so this
888-
# catches both. The "user interrupt" message is a quirk of
889-
# GAP interrupts that result from SIGALRM.
890-
if isinstance(e, AlarmInterrupt) or "user interrupt" in str(e):
891-
# workaround for https://github.com/python/cpython/pull/129276
892-
e.__traceback__ = None
893-
alarm_raised = True
894-
else:
895-
raise
886+
except AlarmInterrupt as e:
887+
e.__traceback__ = None # workaround for https://github.com/python/cpython/pull/129276
888+
alarm_raised = True
896889
finally:
897890
before_cancel_alarm_elapsed = walltime() - start_time
898891
cancel_alarm()

src/sage/libs/gap/util.pyx

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ from posix.dlfcn cimport dlopen, dlclose, dlerror, RTLD_LAZY, RTLD_GLOBAL
1717
from posix.signal cimport sigaction, sigaction_t, sigemptyset
1818

1919
from cpython.exc cimport PyErr_Fetch, PyErr_Restore
20-
from cpython.object cimport Py_LT, Py_LE, Py_EQ, Py_NE, Py_GT, Py_GE
20+
from cpython.object cimport Py_LT, Py_LE, Py_EQ, Py_NE, Py_GT, Py_GE, Py_TYPE
2121
from cpython.ref cimport PyObject, Py_XINCREF, Py_XDECREF
2222

2323
import os
@@ -179,18 +179,24 @@ MakeReadOnlyGlobal("ERROR_OUTPUT");
179179
MakeImmutable(libgap_errout);
180180
"""
181181

182+
182183
# "Global" signal handler info structs. The GAP one we enable/disable
183184
# before/after GAP code. The Sage ones we use to store the existing
184185
# handlers before we do that.
185186
cdef sigaction_t gap_sigint_sa
186187
cdef sigaction_t sage_sigint_sa
187188
cdef sigaction_t sage_sigalrm_sa
189+
cdef int last_signum = 0
190+
188191

189192
cdef void gap_interrupt_asap(int signum) noexcept:
190193
# A wrapper around InterruptExecStat(). This tells GAP to raise an
191194
# error at the next opportunity.
195+
global last_signum
196+
last_signum = signum
192197
InterruptExecStat()
193198

199+
194200
cdef initialize():
195201
"""
196202
Initialize the GAP library, if it hasn't already been
@@ -303,6 +309,7 @@ cdef initialize():
303309
f.close()
304310
gap_eval('SaveWorkspace("{0}")'.format(f.name))
305311

312+
306313
cpdef void gap_sig_on() noexcept:
307314
# Install GAP's own SIGINT handler, typically for the duration of
308315
# some libgap commands. We install it for SIGALRM too so that the
@@ -313,6 +320,7 @@ cpdef void gap_sig_on() noexcept:
313320
sigaction(SIGINT, &gap_sigint_sa, &sage_sigint_sa)
314321
sigaction(SIGALRM, &gap_sigint_sa, &sage_sigalrm_sa)
315322

323+
316324
cpdef void gap_sig_off() noexcept:
317325
# Restore the Sage handlers that were saved & overwritten in
318326
# gap_sig_on(). Better make sure the two are paired correctly!
@@ -321,6 +329,7 @@ cpdef void gap_sig_off() noexcept:
321329
sigaction(SIGINT, &sage_sigint_sa, NULL)
322330
sigaction(SIGALRM, &sage_sigalrm_sa, NULL)
323331

332+
324333
############################################################################
325334
### Evaluate string in GAP #################################################
326335
############################################################################
@@ -434,6 +443,7 @@ cdef Obj gap_eval(str gap_string) except? NULL:
434443
### Error handler ##########################################################
435444
############################################################################
436445

446+
437447
class GAPError(ValueError): # ValueError for historical reasons
438448
"""
439449
Exceptions raised by the GAP library
@@ -478,6 +488,7 @@ cdef void error_handler() noexcept with gil:
478488
cdef PyObject* exc_type = NULL
479489
cdef PyObject* exc_val = NULL
480490
cdef PyObject* exc_tb = NULL
491+
global last_signum
481492

482493
try:
483494
GAP_EnterStack()
@@ -503,19 +514,36 @@ cdef void error_handler() noexcept with gil:
503514
elif not msg:
504515
msg = "an unknown error occurred in GAP"
505516

517+
# PyErr_Fetch gives us a reference to the object, we need to free them
518+
Py_XDECREF(exc_type)
519+
Py_XDECREF(exc_val)
520+
506521
# Raise an exception using PyErr_Restore().
507522
# This way, we can keep any existing traceback object.
508523
# Note that we manually need to deal with refcounts here.
509-
Py_XDECREF(exc_type)
510-
Py_XDECREF(exc_val)
511-
if "user interrupt" in msg:
512-
exc_type = <PyObject*>KeyboardInterrupt
524+
if last_signum:
525+
if last_signum == SIGINT:
526+
exc_type = <PyObject*>KeyboardInterrupt
527+
exc_val_python = KeyboardInterrupt()
528+
exc_val = <PyObject*>exc_val_python
529+
elif last_signum == SIGALRM:
530+
from cysignals.signals import AlarmInterrupt
531+
exc_type = <PyObject*>AlarmInterrupt
532+
exc_val_python = AlarmInterrupt()
533+
exc_val = <PyObject*>exc_val_python
534+
else:
535+
msg = f'{msg}\nunexpected signal value {last_signum} handled, this cannot happen'
536+
exc_type = <PyObject*>GAPError
537+
exc_val = <PyObject*>msg
538+
last_signum = 0
513539
else:
514540
exc_type = <PyObject*>GAPError
515-
exc_val = <PyObject*>msg
541+
exc_val = <PyObject*>msg
542+
516543
Py_XINCREF(exc_type)
517544
Py_XINCREF(exc_val)
518545
PyErr_Restore(exc_type, exc_val, exc_tb)
546+
# as explained in libgap.pyx, this is handled because GAP_Enter's declaration has "except 0"
519547
finally:
520548
# Reset ERROR_OUTPUT with a new text string stream
521549
GAP_EvalStringNoExcept(_reset_error_output_cmd)

0 commit comments

Comments
 (0)