diff --git a/src/sage/doctest/util.py b/src/sage/doctest/util.py index cf0d0133d94..ddc59ddbaa2 100644 --- a/src/sage/doctest/util.py +++ b/src/sage/doctest/util.py @@ -883,9 +883,16 @@ def ensure_interruptible_after(seconds: float, max_wait_after_interrupt: float = try: yield data - except AlarmInterrupt as e: - e.__traceback__ = None # workaround for https://github.com/python/cpython/pull/129276 - alarm_raised = True + except KeyboardInterrupt as e: + # AlarmInterrupt is a subclass of KeyboardInterrupt, so this + # catches both. The "user interrupt" message is a quirk of + # GAP interrupts that result from SIGALRM. + if isinstance(e, AlarmInterrupt) or "user interrupt" in str(e): + # workaround for https://github.com/python/cpython/pull/129276 + e.__traceback__ = None + alarm_raised = True + else: + raise finally: before_cancel_alarm_elapsed = walltime() - start_time cancel_alarm() diff --git a/src/sage/libs/gap/element.pyx b/src/sage/libs/gap/element.pyx index f52a73c2ded..44a9585fc85 100644 --- a/src/sage/libs/gap/element.pyx +++ b/src/sage/libs/gap/element.pyx @@ -16,12 +16,11 @@ elements. For general information about GAP, you should read the # **************************************************************************** from cpython.object cimport Py_EQ, Py_NE, Py_LE, Py_GE, Py_LT, Py_GT -from cysignals.signals cimport sig_on, sig_off from sage.libs.gap.gap_includes cimport * from sage.libs.gap.libgap import libgap from sage.libs.gap.util cimport * -from sage.libs.gap.util import GAPError +from sage.libs.gap.util import GAPError, gap_sig_on, gap_sig_off from sage.libs.gmp.mpz cimport * from sage.libs.gmp.pylong cimport mpz_get_pylong from sage.cpython.string cimport str_to_bytes, char_to_str @@ -260,6 +259,18 @@ cdef Obj make_gap_string(sage_string) except NULL: ### generic construction of GapElements #################################### ############################################################################ + +# Custom identifiers for the types of GAP objects that have associated +# GAP_IsFoo() functions in the libgap API. +cdef enum: + OBJ_TYPE_UND = 0 + OBJ_TYPE_INT = 1 + OBJ_TYPE_FLT = 2 + OBJ_TYPE_CHR = 4 + OBJ_TYPE_STR = 8 + OBJ_TYPE_LST = 16 + OBJ_TYPE_REC = 32 + cdef GapElement make_any_gap_element(parent, Obj obj): """ Return the GAP element wrapper of ``obj``. @@ -296,53 +307,105 @@ cdef GapElement make_any_gap_element(parent, Obj obj): sage: irr[1] 0 """ - cdef int num - + if obj is NULL: + return make_GapElement(parent, obj) + if (obj is GAP_True) or (obj is GAP_False) or (obj is GAP_Fail): + return make_GapElement_Boolean(parent, obj) + + # The object's type, if it can be determined from the libgap API. + # Defaults to "undefined." + cdef int obj_type = OBJ_TYPE_UND + + # The length of obj, if it's a list. Needed to exclude the empty + # list from the string <=> list-of-char equivalence. + cdef unsigned int list_len + + # Plan A: Determine the type of ``obj`` using the libgap API. + # Make all libgap-api calls here, between GAP_Enter/GAP_Leave. + # In particular we avoid any *other* functions that may invoke + # GAP_Enter/GAP_Leave. try: GAP_Enter() - if obj is NULL: - return make_GapElement(parent, obj) - num = TNUM_OBJ(obj) if GAP_IsInt(obj): - return make_GapElement_Integer(parent, obj) + obj_type = OBJ_TYPE_INT elif GAP_IsMacFloat(obj): - return make_GapElement_Float(parent, obj) - elif num == T_CYC: - return make_GapElement_Cyclotomic(parent, obj) - elif num == T_FFE: - return make_GapElement_FiniteField(parent, obj) - elif num == T_RAT: - return make_GapElement_Rational(parent, obj) - elif num == T_BOOL: - return make_GapElement_Boolean(parent, obj) - elif num == T_FUNCTION: - return make_GapElement_Function(parent, obj) - elif num == T_PERM2 or num == T_PERM4: - return make_GapElement_Permutation(parent, obj) - elif GAP_IsRecord(obj): - return make_GapElement_Record(parent, obj) - elif GAP_IsList(obj) and GAP_LenList(obj) == 0: - # Empty lists are lists and not strings in Python - return make_GapElement_List(parent, obj) - elif IsStringConv(obj): - # GAP strings are lists, too. Make sure this comes before non-empty make_GapElement_List - return make_GapElement_String(parent, obj) + obj_type = OBJ_TYPE_FLT + elif GAP_IsChar(obj): + obj_type = OBJ_TYPE_CHR + elif GAP_IsString(obj): + # Efficient representation only, returns False on lists. + obj_type = OBJ_TYPE_STR elif GAP_IsList(obj): - return make_GapElement_List(parent, obj) - elif GAP_ValueOfChar(obj) != -1: - ch = make_GapElement(parent, obj).IntChar().sage() - return make_GapElement_String(parent, make_gap_string(chr(ch))) - result = make_GapElement(parent, obj) - if num == T_POSOBJ: - if result.IsZmodnZObj(): - return make_GapElement_IntegerMod(parent, obj) - if num == T_COMOBJ: - if result.IsRing(): - return make_GapElement_Ring(parent, obj) - return result + obj_type = OBJ_TYPE_LST + list_len = GAP_LenList(obj) + elif GAP_IsRecord(obj): + obj_type = OBJ_TYPE_REC finally: GAP_Leave() + if obj_type == OBJ_TYPE_INT: + return make_GapElement_Integer(parent, obj) + elif obj_type == OBJ_TYPE_FLT: + return make_GapElement_Float(parent, obj) + elif obj_type == OBJ_TYPE_CHR: + # IntChar is a GAP function call; these do their own GAP_Enter + # and GAP_Leave, as does make_gap_string(). + ch = make_GapElement(parent, obj).IntChar().sage() + return make_GapElement_String(parent, make_gap_string(chr(ch))) + elif obj_type == OBJ_TYPE_STR: + return make_GapElement_String(parent, obj) + elif obj_type == OBJ_TYPE_LST: + result = make_GapElement_List(parent, obj) + if list_len > 0 and result.IsString(): + # IsString() thinks a list-of-char is a string, but + # GAP_IsString() does not. For backwards compatibility we + # convert list-of-char to an efficient string here, and + # return an instance of the more-specific class. The + # length check is necessary to prevent the conversion of + # all empty lists to empty strings. A related problem is + # that you cannot construct an actual list-of-char here + # if you want to. + result = make_GapElement_String(parent, obj) + result.ConvertToStringRep() + return result + else: + return result + elif obj_type == OBJ_TYPE_REC: + return make_GapElement_Record(parent, obj) + + # Plan B: Use the "private" API to determine the type of ``obj``. + # This is not kosher, but there are no such functions in + # libgap-api.h, and this is much faster than using the + # corresponding IsFoo() functions on a generic GAPElement. + # TNUM_OBJ involves a series of macros, none of which do any + # memory management or error handling, so this is safe to do + # outside of GAP_Enter / GAP_Leave for now. + cdef int num = TNUM_OBJ(obj) + if num == T_RAT: + return make_GapElement_Rational(parent, obj) + elif num == T_FUNCTION: + return make_GapElement_Function(parent, obj) + elif num == T_CYC: + return make_GapElement_Cyclotomic(parent, obj) + elif num == T_FFE: + return make_GapElement_FiniteField(parent, obj) + elif num == T_PERM2 or num == T_PERM4: + return make_GapElement_Permutation(parent, obj) + + # Plan C: Make a generic element, and then use GAP functions to + # check if it's an IntegerMod or a Ring. This is our penultimate + # resort due to how slow it is. + result = make_GapElement(parent, obj) + if num == T_POSOBJ: + if result.IsZmodnZObj(): + return make_GapElement_IntegerMod(parent, obj) + if num == T_COMOBJ: + if result.IsRing(): + return make_GapElement_Ring(parent, obj) + + # Plan D: return the generic element. + return result + ############################################################################ ### GapElement ############################################################# @@ -935,13 +998,15 @@ cdef class GapElement(RingElement): if self._compare_by_id: return id(self) == id(other) cdef GapElement c_other = other - sig_on() + try: + gap_sig_on() GAP_Enter() return GAP_EQ(self.value, c_other.value) finally: GAP_Leave() - sig_off() + gap_sig_off() + cdef bint _compare_less(self, Element other) except -2: """ @@ -957,13 +1022,15 @@ cdef class GapElement(RingElement): if self._compare_by_id: return id(self) < id(other) cdef GapElement c_other = other - sig_on() + try: + gap_sig_on() GAP_Enter() return GAP_LT(self.value, c_other.value) finally: GAP_Leave() - sig_off() + gap_sig_off() + cpdef _add_(self, right): r""" @@ -986,12 +1053,12 @@ cdef class GapElement(RingElement): """ cdef Obj result try: - sig_GAP_Enter() - sig_on() + gap_sig_on() + GAP_Enter() result = GAP_SUM(self.value, (right).value) - sig_off() finally: GAP_Leave() + gap_sig_off() return make_any_gap_element(self.parent(), result) cpdef _sub_(self, right): @@ -1014,12 +1081,12 @@ cdef class GapElement(RingElement): """ cdef Obj result try: - sig_GAP_Enter() - sig_on() + gap_sig_on() + GAP_Enter() result = GAP_DIFF(self.value, (right).value) - sig_off() finally: GAP_Leave() + gap_sig_off() return make_any_gap_element(self.parent(), result) cpdef _mul_(self, right): @@ -1043,12 +1110,12 @@ cdef class GapElement(RingElement): """ cdef Obj result try: - sig_GAP_Enter() - sig_on() + gap_sig_on() + GAP_Enter() result = GAP_PROD(self.value, (right).value) - sig_off() finally: GAP_Leave() + gap_sig_off() return make_any_gap_element(self.parent(), result) cpdef _div_(self, right): @@ -1077,12 +1144,12 @@ cdef class GapElement(RingElement): """ cdef Obj result try: - sig_GAP_Enter() - sig_on() + gap_sig_on() + GAP_Enter() result = GAP_QUO(self.value, (right).value) - sig_off() finally: GAP_Leave() + gap_sig_off() return make_any_gap_element(self.parent(), result) cpdef _mod_(self, right): @@ -1104,12 +1171,12 @@ cdef class GapElement(RingElement): """ cdef Obj result try: - sig_GAP_Enter() - sig_on() + gap_sig_on() + GAP_Enter() result = GAP_MOD(self.value, (right).value) - sig_off() finally: GAP_Leave() + gap_sig_off() return make_any_gap_element(self.parent(), result) cpdef _pow_(self, other): @@ -1132,11 +1199,21 @@ cdef class GapElement(RingElement): TESTS: - Check that this can be interrupted gracefully:: + Check that this method can be interrupted gracefully. The + repeats build confidence that everything remains sane after an + interruption:: - sage: a, b = libgap.GL(1000, 3).GeneratorsOfGroup(); g = a * b + sage: # long time + sage: a, b = libgap.GL(1000, 3).GeneratorsOfGroup() + sage: g = a * b + sage: e = libgap(2^400000) sage: from sage.doctest.util import ensure_interruptible_after - sage: with ensure_interruptible_after(0.5): g ^ (2 ^ 10000) + sage: for _ in range(10): + ....: with ensure_interruptible_after(0.1, max_wait_after_interrupt=5): + ....: g._pow_(e) + + Check that a :exc:`sage.libs.gap.util.GAPError` is raised + under error conditions:: sage: libgap.CyclicGroup(2) ^ 2 Traceback (most recent call last): @@ -1144,6 +1221,8 @@ cdef class GapElement(RingElement): GAPError: Error, no method found! Error, no 1st choice method found for `^' on 2 arguments + :: + sage: libgap(3) ^ Infinity Traceback (most recent call last): ... @@ -1151,12 +1230,12 @@ cdef class GapElement(RingElement): method found for `InverseMutable' on 1 arguments """ try: - sig_GAP_Enter() - sig_on() + gap_sig_on() + GAP_Enter() # GAPError raised from here result = GAP_POW(self.value, (other).value) - sig_off() finally: GAP_Leave() + gap_sig_off() return make_any_gap_element(self._parent, result) cpdef _pow_int(self, other): @@ -1198,7 +1277,11 @@ cdef class GapElement(RingElement): sage: libgap.eval('3/2').is_list() False """ - return GAP_IsList(self.value) + try: + GAP_Enter() + return GAP_IsList(self.value) + finally: + GAP_Leave() def is_record(self): r""" @@ -1213,7 +1296,11 @@ cdef class GapElement(RingElement): sage: libgap.eval('rec(a:=1, b:=3)').is_record() True """ - return GAP_IsRecord(self.value) + try: + GAP_Enter() + return GAP_IsRecord(self.value) + finally: + GAP_Leave() cpdef is_bool(self): r""" @@ -1242,7 +1329,11 @@ cdef class GapElement(RingElement): sage: libgap('this is a string').is_string() True """ - return IS_STRING(self.value) + try: + GAP_Enter() + return GAP_IsString(self.value) + finally: + GAP_Leave() def is_permutation(self): r""" @@ -1376,11 +1467,6 @@ cdef class GapElement(RingElement): x = R.gen() return x**val * R(num) / R(den) - elif self.IsList(): - # May be a list-like collection of some other type of GapElements - # that we can convert - return [item.sage() for item in self.AsList()] - elif self.IsFreeGroup(): from sage.groups.free_group import FreeGroup_class names = tuple(str(g) for g in self.GeneratorsOfGroup()) @@ -2498,36 +2584,34 @@ cdef class GapElement_Function(GapElement): cdef Obj result = NULL cdef Obj arg_list cdef int n = len(args) - cdef volatile Obj v2 - - if n > 0 and n <= 3: - libgap = self.parent() - a = [x if isinstance(x, GapElement) else libgap(x) for x in args] + cdef Obj a[3] + + if n <= 3: + if not all(isinstance(x, GapElement) for x in args): + libgap = self.parent() + args = tuple(x if isinstance(x, GapElement) else libgap(x) for x in args) + for i in range(n): + x = args[i] + a[i] = (x).value + else: + arg_list = make_gap_list(args) try: - sig_GAP_Enter() - sig_on() + gap_sig_on() + GAP_Enter() if n == 0: result = GAP_CallFunc0Args(self.value) elif n == 1: - result = GAP_CallFunc1Args(self.value, - (a[0]).value) + result = GAP_CallFunc1Args(self.value, a[0]) elif n == 2: - result = GAP_CallFunc2Args(self.value, - (a[0]).value, - (a[1]).value) + result = GAP_CallFunc2Args(self.value, a[0], a[1]) elif n == 3: - v2 = (a[2]).value - result = GAP_CallFunc3Args(self.value, - (a[0]).value, - (a[1]).value, - v2) + result = GAP_CallFunc3Args(self.value, a[0], a[1], a[2]) else: - arg_list = make_gap_list(args) result = GAP_CallFuncList(self.value, arg_list) - sig_off() finally: GAP_Leave() + gap_sig_off() if result == NULL: # We called a procedure that does not return anything return None @@ -3151,13 +3235,14 @@ cdef class GapElement_Record(GapElement): """ cdef UInt i = self.record_name_to_index(name) cdef Obj result - sig_on() + try: + gap_sig_on() GAP_Enter() result = ELM_REC(self.value, i) finally: GAP_Leave() - sig_off() + gap_sig_off() return make_any_gap_element(self.parent(), result) def sage(self): diff --git a/src/sage/libs/gap/gap_includes.pxd b/src/sage/libs/gap/gap_includes.pxd index 35107d3d4a0..e3b05d88ba6 100644 --- a/src/sage/libs/gap/gap_includes.pxd +++ b/src/sage/libs/gap/gap_includes.pxd @@ -27,9 +27,6 @@ cdef extern from "gap/calls.h" nogil: cdef extern from "gap/libgap-api.h" nogil: - """ - #define sig_GAP_Enter() {int t = GAP_Enter(); if (!t) sig_error();} - """ ctypedef void (*GAP_CallbackFunc)() void GAP_Initialize(int argc, char ** argv, GAP_CallbackFunc markBagsCallback, GAP_CallbackFunc errorCallback, @@ -40,7 +37,6 @@ cdef extern from "gap/libgap-api.h" nogil: cdef void GAP_EnterStack() cdef void GAP_LeaveStack() cdef int GAP_Enter() except 0 - cdef void sig_GAP_Enter() cdef void GAP_Leave() cdef int GAP_Error_Setjmp() except 0 @@ -59,6 +55,7 @@ cdef extern from "gap/libgap-api.h" nogil: cdef Obj GAP_True cdef Obj GAP_False + cdef Obj GAP_Fail Obj GAP_CallFuncList(Obj func, Obj args); Obj GAP_CallFuncArray(Obj func, UInt narg, Obj * args); @@ -91,7 +88,7 @@ cdef extern from "gap/libgap-api.h" nogil: char* GAP_CSTR_STRING(Obj list) Obj GAP_MakeStringWithLen(const char* buf, UInt len) - Int GAP_ValueOfChar(Obj obj) + bint GAP_IsChar(Obj obj) cdef extern from "gap/lists.h" nogil: @@ -143,11 +140,13 @@ cdef extern from "gap/records.h" nogil: cdef extern from "gap/stringobj.h" nogil: - bint IS_STRING(Obj obj) - bint IsStringConv(Obj obj) Obj NEW_STRING(Int) +cdef extern from "gap/stats.h" nogil: + void InterruptExecStat() + + cdef extern from "" nogil: """ /* Hack: Cython 3.0 automatically includes , which diff --git a/src/sage/libs/gap/libgap.pyx b/src/sage/libs/gap/libgap.pyx index f5b58a929f3..bcf94a65f27 100644 --- a/src/sage/libs/gap/libgap.pyx +++ b/src/sage/libs/gap/libgap.pyx @@ -160,20 +160,124 @@ using the recursive expansion of the Using the GAP C library from Cython =================================== -.. TODO:: Expand the following text - - We are using the GAP API provided by the GAP project since - GAP 4.10. +We are using the GAP API provided by the GAP project since GAP 4.10. + +Calls to the GAP C library (functions declared in ``libgap-api.h``) +should be sandwiched between calls to ``GAP_Enter()`` and +``GAP_Leave()``. These are macros defined in ``libgap-api.h`` and must +be used carefully because ``GAP_Enter()`` is defined as two function +calls in succession without braces. The first thing that +``GAP_Enter()`` does is a ``setjmp()`` which plays an important role +in handling errors. The return value from ``GAP_Enter()`` is non-zero +(success) the first time around, and if an error occurs, execution +"jumps" back to ``GAP_Enter()``, this time with a return value of zero +(failure). Due to these quirks, naive attempts to handle the return +value of ``GAP_Enter()`` are doomed to fail. The correct pattern to +use is:: + + try: + GAP_Enter() + # further calls to libgap + finally: + GAP_Leave() + +How this works is subtle. When GAP is initialized, we install an +``error_handler()`` callback that GAP invokes on error. This function +sets a python exception using ``PyErr_Restore()``, but so long as we +remain in C, this exception will not actually be raised. When +``error_handler()`` finishes executing, control returns to GAP which +then jumps back to the previous ``GAP_Enter()``. It is at this point +that we need to raise the (already set) exception, to prevent +re-executing the code that caused an error. To facilitate this, +``GAP_Enter()`` is wrapped by Cython, and the wrapper is qualified +with ``except 0``. This tells Cython to treat a return value of zero +as an error, and raise an exception if an exception is set. (One will +be set if there was an error because our ``error_handler()`` sets +it). Here is a real example:: + + cpdef void crash_and_burn() except *: + x = libgap({'a': 1, 'b': 2}) + cdef unsigned int xlen + try: + GAP_Enter() + xlen = GAP_LenList((x).value) + finally: + GAP_Leave() + print(xlen) + +The call to ``GAP_LenList()`` is an error in this case, because +``x.value`` is a GAP record, not a GAP list. In any case, what happens +is, + +#. We call the ``GAP_Enter()`` Cython wrapper, which invokes the + macro, and additionally generates some C code to raise an + exception if that return value is zero (error). But this is the + first pass, so for now the macro returns a non-zero (success) + value. +#. We call ``GAP_LenList(x.value)``, which is an error. +#. GAP invokes our ``error_handler()``, which creates a + :exc:`sage.libs.gap.util.GAPError`, and sets it active. +#. Control returns to GAP. +#. GAP jumps back to ``GAP_Enter()``. +#. The error branch of ``GAP_Enter()`` is executed. In other words + we proceed from ``GAP_Enter()`` as if it returned zero (error). +#. An exception is raised, because the ``except 0`` qualifier on the + Cython wrapper for ``GAP_Enter()`` specifically checks for zero + and raises any exceptions in that case. +#. Finally, ``GAP_Leave()`` is called to clean up. In a more + realistic example where failure is not guaranteed, this would + also have been run to clean up if no errors were raised. + +Another unusual aspect of the libgap interface is its signal +handling. Typically, cysignals' ``sig_on()`` and ``sig_off()`` +functions are used to wrap code that may take a long time, and as a +result, may need to be interrupted with Ctrl-C. However, it is +possible that interrupting a function execution at an arbitrary +location will lead to inconsistent state. Internally, GAP provides a +mechanism using ``InterruptExecStat``, which sets a flag that tells +GAP to gracefully exit with an error as early as possible. We make use +of this internal mechanism to prevent segmentation faults when GAP +functions are interrupted. + +Specifically, we install GAP's own ``SIGINT`` handler (to catch +Ctrl-C) before executing any long-running GAP code, and then later +reinstall the original handler when the GAP code has finished. This is +accomplished using the suggestively-named ``gap_sig_on()`` and +``gap_sig_off()`` functions. After you have called ``gap_sig_on()``, +if GAP receives Ctrl-C, it will invoke our custom ``error_handler()`` +that will set a :exc:`KeyboardInterrupt` containing the phrase "user +interrupt". Eventually (as explained in the preceding paragraphs), +control will jump back to the Cython wrapper for ``GAP_Enter()``, and +this exception will be raised. + +The safest pattern to use for interruptible libgap code is:: + + try: + gap_sig_on() + GAP_Enter() + # further calls to libgap + finally: + GAP_Leave() + gap_sig_off() + +Before you attempt to change any of this, please make sure that +you understand the issues that it is intended to fix, e.g. + +* https://github.com/sagemath/sage/issues/37026 +* https://trofi.github.io/posts/312-the-sagemath-saga.html +* https://github.com/sagemath/sage/pull/40585 +* https://github.com/sagemath/sage/pull/40594 +* https://github.com/sagemath/sage/issues/40598 AUTHORS: - - William Stein, Robert Miller (2009-06-23): first version - - Volker Braun, Dmitrii Pasechnik, Ivan Andrus (2011-03-25, Sage Days 29): - almost complete rewrite; first usable version. - - Volker Braun (2012-08-28, GAP/Singular workshop): update to - gap-4.5.5, make it ready for public consumption. - - Dima Pasechnik (2018-09-18, GAP Days): started the port to native - libgap API +- William Stein, Robert Miller (2009-06-23): first version +- Volker Braun, Dmitrii Pasechnik, Ivan Andrus (2011-03-25, Sage Days 29): + almost complete rewrite; first usable version. +- Volker Braun (2012-08-28, GAP/Singular workshop): update to + gap-4.5.5, make it ready for public consumption. +- Dima Pasechnik (2018-09-18, GAP Days): started the port to native + libgap API """ ############################################################################### diff --git a/src/sage/libs/gap/meson.build b/src/sage/libs/gap/meson.build index def07898f4c..94170dca488 100644 --- a/src/sage/libs/gap/meson.build +++ b/src/sage/libs/gap/meson.build @@ -33,7 +33,7 @@ foreach name, pyx : extension_data subdir: 'sage/libs/gap', install: true, include_directories: [inc_cpython, inc_rings], - dependencies: [py_dep, cysignals, gap, gmp], + dependencies: [py_dep, gap, gmp], ) endforeach diff --git a/src/sage/libs/gap/util.pyx b/src/sage/libs/gap/util.pyx index a5e3ad8602a..f23879040b0 100644 --- a/src/sage/libs/gap/util.pyx +++ b/src/sage/libs/gap/util.pyx @@ -12,13 +12,13 @@ Utility functions for GAP # http://www.gnu.org/licenses/ #***************************************************************************** -from libc.signal cimport signal, SIGCHLD, SIG_DFL +from libc.signal cimport signal, SIGALRM, SIGCHLD, SIG_DFL, SIGINT from posix.dlfcn cimport dlopen, dlclose, dlerror, RTLD_LAZY, RTLD_GLOBAL +from posix.signal cimport sigaction, sigaction_t, sigemptyset from cpython.exc cimport PyErr_Fetch, PyErr_Restore from cpython.object cimport Py_LT, Py_LE, Py_EQ, Py_NE, Py_GT, Py_GE from cpython.ref cimport PyObject, Py_XINCREF, Py_XDECREF -from cysignals.signals cimport sig_on, sig_off import os import warnings @@ -130,7 +130,6 @@ cdef void reference_obj(Obj obj) noexcept: """ cdef ObjWrapper wrapped = wrap_obj(obj) global owned_objects_refcount -# print("reference_obj called "+ crepr(obj) +"\n") if wrapped in owned_objects_refcount: owned_objects_refcount[wrapped] += 1 else: @@ -180,6 +179,17 @@ MakeReadOnlyGlobal("ERROR_OUTPUT"); MakeImmutable(libgap_errout); """ +# "Global" signal handler info structs. The GAP one we enable/disable +# before/after GAP code. The Sage ones we use to store the existing +# handlers before we do that. +cdef sigaction_t gap_sigint_sa +cdef sigaction_t sage_sigint_sa +cdef sigaction_t sage_sigalrm_sa + +cdef void gap_interrupt_asap(int signum) noexcept: + # A wrapper around InterruptExecStat(). This tells GAP to raise an + # error at the next opportunity. + InterruptExecStat() cdef initialize(): """ @@ -258,11 +268,21 @@ cdef initialize(): argv[argc] = NULL - sig_on() - # Initialize GAP but disable their SIGINT handler + # Initialize GAP but disable its signal handlers: we only want + # them to be invoked while libgap code is executing, whereas the + # default would enable them globally. We will explicitly wrap GAP + # computations in gap_sig_on() and gap_sig_off() instead. GAP_Initialize(argc, argv, gasman_callback, error_handler, handleSignals=False) - sig_off() + + # Configure a SIGINT handler (which can be enabled by calling + # gap_sig_on) to run InterruptExecStat. This is essentially GAP's + # own SIGINT handler (but without the double-Ctrl-C behavior), and + # is less crashy than when we mix cysignals with GAP code. + global gap_sigint_sa + gap_sigint_sa.sa_handler = gap_interrupt_asap + sigemptyset(&(gap_sigint_sa.sa_mask)) + gap_sigint_sa.sa_flags = 0; # Disable GAP's SIGCHLD handler ChildStatusChanged(), which calls # waitpid() on random child processes. @@ -283,6 +303,23 @@ cdef initialize(): f.close() gap_eval('SaveWorkspace("{0}")'.format(f.name)) +cpdef void gap_sig_on() noexcept: + # Install GAP's own SIGINT handler, typically for the duration of + # some libgap commands. We install it for SIGALRM too so that the + # doctest runner can use alarm() to interrupt it. + global gap_sigint_sa + global sage_sigint_sa + global sage_sigalrm_sa + sigaction(SIGINT, &gap_sigint_sa, &sage_sigint_sa) + sigaction(SIGALRM, &gap_sigint_sa, &sage_sigalrm_sa) + +cpdef void gap_sig_off() noexcept: + # Restore the Sage handlers that were saved & overwritten in + # gap_sig_on(). Better make sure the two are paired correctly! + global sage_sigint_sa + global sage_sigalrm_sa + sigaction(SIGINT, &sage_sigint_sa, NULL) + sigaction(SIGALRM, &sage_sigalrm_sa, NULL) ############################################################################ ### Evaluate string in GAP ################################################# @@ -355,8 +392,9 @@ cdef Obj gap_eval(str gap_string) except? NULL: # so that Cython doesn't deallocate it before GAP is done with # its contents. cmd = str_to_bytes(gap_string + ';\n') - sig_on() + try: + gap_sig_on() GAP_Enter() result = GAP_EvalString(cmd) # We can assume that the result object is a GAP PList (plain list) @@ -389,7 +427,7 @@ cdef Obj gap_eval(str gap_string) except? NULL: return GAP_ElmList(result, 2) finally: GAP_Leave() - sig_off() + gap_sig_off() ############################################################################ @@ -470,7 +508,10 @@ cdef void error_handler() noexcept with gil: # Note that we manually need to deal with refcounts here. Py_XDECREF(exc_type) Py_XDECREF(exc_val) - exc_type = GAPError + if "user interrupt" in msg: + exc_type = KeyboardInterrupt + else: + exc_type = GAPError exc_val = msg Py_XINCREF(exc_type) Py_XINCREF(exc_val) diff --git a/src/sage/rings/complex_arb.pyx b/src/sage/rings/complex_arb.pyx index b6f3ed191e8..0cab5ba87e9 100644 --- a/src/sage/rings/complex_arb.pyx +++ b/src/sage/rings/complex_arb.pyx @@ -1187,8 +1187,8 @@ class ComplexBallField(UniqueRepresentation, sage.rings.abc.ComplexBallField): [0.4596976941318602825990633926 +/- ...e-29] sage: from sage.doctest.util import ensure_interruptible_after - sage: with ensure_interruptible_after(0.1): - ....: C = ComplexBallField(1000000) + sage: C = ComplexBallField(1000000) + sage: with ensure_interruptible_after(0.1, max_wait_after_interrupt=5): ....: C.integral(lambda x, _: x.cos() * x.sin(), 0, 1) """ cdef IntegrationContext ctx = IntegrationContext()