Trac #24986: Inconsistent mpz_t state after interrupted sig_realloc()

=== TODO ===

||☐||Use `sig_occurred()` to check whether an exception from Cysignals
is currently being handled while in `Integer.tp_dealloc`. If so, assume
that the state of the object's mpz struct may not be consistent, so do
not call `mpz_clear` on it and do not place it back in the free pool.||
||☐||Don't forget to re-enable the test that was disabled in #25137 in
order to test that this is fixed.||

----
As discussed on sage-devel, I'm fairly consistently (roughly 9 times out
of 10) getting the following failure on Cygwin:

{{{
sage -t --warn-long 164.8 src/sage/structure/coerce_actions.pyx
**********************************************************************
File "src/sage/structure/coerce_actions.pyx", line 786, in
sage.structure.coerce_actions.IntegerMulAction._repr_name_
Failed example:
    IntegerMulAction(ZZ, GF5)
Expected:
    Left Integer Multiplication by Integer Ring on Finite Field of size
5
Got:
    Left Integer Multiplication by Integer Ring on Finite Field of size
1
**********************************************************************
1 item had failures:
   1 of   4 in
sage.structure.coerce_actions.IntegerMulAction._repr_name_
    [143 tests, 1 failure, 3.30 s]
----------------------------------------------------------------------
sage -t --warn-long 164.8 src/sage/structure/coerce_actions.pyx  # 1
doctest failed
}}}

Obviously Sage doesn't even allow creation of an order 1 field.  In
fact, I traced the cause of this to a ''specific'' line in
`FiniteFieldFactory.create_key_and_extra_args` where, by chance, an
`Integer` with a value of `1` is constructed (using `fast_tp_new`) whose
`(mp_limb*)(Integer.value._mp_d)` member is assigned the same address as
the `_mp_d` of the `Integer` that happens to hold the field's order.

The result is that the `order` is then set to `1` as well.  This happens
''after'' the check that `order>1` so creation of the field still
succeeds.  Clearly there is a subtle bug either in `fast_tp_new`, or in
the memory allocator itself.

URL: https://trac.sagemath.org/24986
Reported by: embray
Ticket author(s): Jeroen Demeyer
Reviewer(s): Erik Bray

Author: Release Manager

src/sage/doctest/forker.py

@@ -673,6 +673,16 @@ def compiler(example):
                 raise
             except BaseException:
                 exception = sys.exc_info()
+                # On Python 2, the exception lives in sys.exc_info() as
+                # long we are in the same stack frame. To ensure that
+                # sig_occurred() works correctly, we need to clear the
+                # exception. This is not an issue on Python 3, where the
+                # exception is cleared as soon as we are outside of the
+                # "except" clause.
+                try:
+                    sys.exc_clear()
+                except AttributeError:
+                    pass  # Python 3
             finally:
                 if self.debugger is not None:
                     self.debugger.set_continue() # ==== Example Finished ====
@@ -699,8 +709,7 @@ def compiler(example):
 
             # The example raised an exception: check if it was expected.
             else:
-                exc_info = exception
-                exc_msg = traceback.format_exception_only(*exc_info[:2])[-1]
+                exc_msg = traceback.format_exception_only(*exception[:2])[-1]
 
                 if six.PY3 and example.exc_msg is not None:
                     # On Python 3 the exception repr often includes the
@@ -709,7 +718,7 @@ def compiler(example):
                     # normalize Python 3 exceptions to match tests written to
                     # Python 2
                     # See https://trac.sagemath.org/ticket/24271
-                    exc_cls = exc_info[0]
+                    exc_cls = exception[0]
                     exc_name = exc_cls.__name__
                     if exc_cls.__module__:
                         exc_fullname = (exc_cls.__module__ + '.' +
@@ -736,7 +745,7 @@ def compiler(example):
                                     break
 
                 if not quiet:
-                    got += doctest._exception_traceback(exc_info)
+                    got += doctest._exception_traceback(exception)
 
                 # If `example.exc_msg` is None, then we weren't expecting
                 # an exception.
@@ -770,7 +779,7 @@ def compiler(example):
             elif outcome is BOOM:
                 if not quiet:
                     self.report_unexpected_exception(out, test, example,
-                                                     exc_info)
+                                                     exception)
                 failures += 1
             else:
                 assert False, ("unknown outcome", outcome)

src/sage/rings/integer.pyx

@@ -144,11 +144,10 @@ cimport cython
 from libc.math cimport (ldexp, sqrt as sqrt_double, log as log_c,
         ceil as ceil_c, isnan)
 from libc.string cimport memcpy
-cdef extern from "<limits.h>":
-    const long LONG_MAX  # Work around https://github.com/cython/cython/pull/2016
+from libc.limits cimport LONG_MAX
 
 from cysignals.memory cimport check_allocarray, check_malloc, sig_free
-from cysignals.signals cimport sig_on, sig_off, sig_check
+from cysignals.signals cimport sig_on, sig_off, sig_check, sig_occurred
 
 import operator
 import sys
@@ -7305,39 +7304,41 @@ cdef PyObject* fast_tp_new(type t, args, kwds) except NULL:
 
     return new
 
-cdef void fast_tp_dealloc(PyObject* o):
 
+cdef void fast_tp_dealloc(PyObject* o):
     # If there is room in the pool for a used integer object,
     # then put it in rather than deallocating it.
-
     global integer_pool, integer_pool_count
 
     cdef mpz_ptr o_mpz = <mpz_ptr>((<Integer>o).value)
 
-    if integer_pool_count < integer_pool_size:
+    # If we are recovering from an interrupt, throw the mpz_t away
+    # without recycling or freeing it because it might be in an
+    # inconsistent state (see Trac #24986).
+    if sig_occurred() is NULL:
+        if integer_pool_count < integer_pool_size:
+            # Here we free any extra memory used by the mpz_t by
+            # setting it to a single limb.
+            if o_mpz._mp_alloc > 10:
+                _mpz_realloc(o_mpz, 1)
 
-        # Here we free any extra memory used by the mpz_t by
-        # setting it to a single limb.
-        if o_mpz._mp_alloc > 10:
-            _mpz_realloc(o_mpz, 1)
+            # It's cheap to zero out an integer, so do it here.
+            o_mpz._mp_size = 0
 
-        # It's cheap to zero out an integer, so do it here.
-        o_mpz._mp_size = 0
+            # And add it to the pool.
+            integer_pool[integer_pool_count] = o
+            integer_pool_count += 1
+            return
 
-        # And add it to the pool.
-        integer_pool[integer_pool_count] = o
-        integer_pool_count += 1
-        return
-
-    # Again, we move to the mpz_t and clear it. As in fast_tp_new,
-    # we free the memory directly.
-    sig_free(o_mpz._mp_d)
+        # No space in the pool, so just free the mpz_t.
+        sig_free(o_mpz._mp_d)
 
     # Free the object. This assumes that Py_TPFLAGS_HAVE_GC is not
     # set. If it was set another free function would need to be
     # called.
     PyObject_Free(o)
 
+
 from sage.misc.allocator cimport hook_tp_functions
 cdef hook_fast_tp_functions():
     """

src/sage/structure/coerce_actions.pyx

@@ -736,10 +736,17 @@ cdef class IntegerMulAction(IntegerAction):
 
         Check that large multiplications can be interrupted::
 
-            sage: alarm(0.5); (2^(10^7)) * P  # not tested; see trac:#24986
+            sage: alarm(0.5); (2^(10^6)) * P
             Traceback (most recent call last):
             ...
             AlarmInterrupt
+
+        Verify that cysignals correctly detects that the above
+        exception has been handled::
+
+            sage: from cysignals.tests import print_sig_occurred
+            sage: print_sig_occurred()
+            No current exception
         """
         cdef int err = 0
         cdef long n_long