Explicitly check signum in GAP error handler

[user202729]

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

• 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

[github-actions]

Documentation preview for this PR (built with commit 4010185; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[user202729]

I don't understand the remark about wasting memory in exceptional case however.

[orlitzky]

I don't understand the remark about wasting memory in exceptional case however.

The old version was constructing two exception objects in global scope (so, effectively, when sage starts) and then never destroying them. It's irrelevant now.

[orlitzky]

I started poking at the memory usage because I was confused about why PyErr_Restore sometimes takes a string and sometimes an exception. I don't think this has anything to do with this PR, but the memory usage balloons if we catch these alarms in a loop. Here is a quick script:

import gc
import psutil
from sage.doctest.util import ensure_interruptible_after

def vmem_used_in_mib():
    return psutil.virtual_memory().used / 1024.0**2

a, b = libgap.GL(1000, 3).GeneratorsOfGroup()
g = a * b
e = libgap(2 ^ 400000)
print(f"vmem used before: {vmem_used_in_mib()}")
for _ in range(5000):
    with ensure_interruptible_after(0.1, max_wait_after_interrupt=5):
        g^e
    gc.collect()
print(f"vmem used after: {vmem_used_in_mib()}")

And running it, I get output like:

$ sage example.sage
vmem used before: 1226.07421875000
vmem used after: 2362.39843750000

$ sage example.sage
vmem used before: 651.539062500000
vmem used after: 2477.03125000000

(The "used" memory reported by psutil is nonsense, but you can still wonder why it grows in some cases and does not grow in others. The OS shouldn't need to allocate more memory ad infinitum if each loop iteration cleans up after itself.)

[orlitzky]

I think the logic is cleaner without the special case for the signum that can never happen, e.g.

if last_signum == SIGINT:
    exc_type = <PyObject*>KeyboardInterrupt
    exc_val_python = KeyboardInterrupt()
    exc_val = <PyObject*>exc_val_python
elif last_signum == SIGALRM:
    from cysignals.signals import AlarmInterrupt
    exc_type = <PyObject*>AlarmInterrupt
    exc_val_python = AlarmInterrupt()
    exc_val = <PyObject*>exc_val_python
else:
    exc_type = <PyObject*>GAPError
    exc_val = <PyObject*>msg
last_signum = 0

(it doesn't hurt to set last_signum = 0 if it is zero already, this isn't a tight loop.) But I'm not picky if you want to keep it the way it is.

[user202729]

maybe it's just because gap itself leaks memory?

[orlitzky]

maybe it's just because gap itself leaks memory?

The GAP_Enter() / GAP_Leave() routine should be able to clean up any memory that was allocated. Valgrind will probably be needed.

It occurred to me that you could even keep the "this should never happen" message in the example I gave:

else:
    if not last_signum:
        msg = "..."
    exc_type = <PyObject*>GAPError
    exc_val = <PyObject*>msg

so the trade-off is between duplicating that "else" block, and setting last_signum=0 unnecessarily. I'll set to positive review in any case since it works as-is.

[user202729]

about PyErr_Restore taking a string, I have no idea either, but the previous code does that as well, and there are some remarks scattered online that if PyErr_SetString is used then PyErr_Fetch returns a string, and from the documentation we know that PyErr_Restore ought to be able to take what PyErr_Fetch returns.