Opened 2 years ago

Closed 2 years ago

Last modified 10 months ago

#23199 closed defect (fixed)

missing sig_on/off in symbolic arithmetics

Reported by: rws Owned by:
Priority: major Milestone: sage-8.1
Component: symbolics Keywords:
Cc: Merged in:
Authors: Ralf Stephan Reviewers: Jeroen Demeyer, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: af5ef8b (Commits) Commit:
Dependencies: Stopgaps:

Description

Throwing exceptions from Pynac when Python raises some will crash Sage in a Pynac prototype because signal guards are missing in exception.pyx.

Change History (17)

comment:1 Changed 2 years ago by rws

  • Branch set to u/rws/missing_sig_on_off_in_symbolic_arithmetics

comment:2 Changed 2 years ago by rws

  • Authors set to Ralf Stephan
  • Commit set to aa71b3d341fc5cb709b9df64ee30217758f87fbe
  • Status changed from new to needs_review

New commits:

aa71b3d23199: missing sig_on/off in symbolic arithmetics

comment:3 Changed 2 years ago by rws

  • Status changed from needs_review to needs_work

comment:4 Changed 2 years ago by rws

  • Branch changed from u/rws/missing_sig_on_off_in_symbolic_arithmetics to u/rws/23199

comment:5 Changed 2 years ago by jdemeyer

  • Commit changed from aa71b3d341fc5cb709b9df64ee30217758f87fbe to fa449e4ef165fdb4357a78d8f053c95a77ac6f0b

You can simplify

try:
    [...]
    X
except (...):
    X
    raise

to

try:
    [...]
finally:
    X

New commits:

fa449e423199: missing sig_on/off in symbolic arithmetics

comment:6 Changed 2 years ago by rws

  • Branch changed from u/rws/23199 to u/rws/23199-1

comment:7 Changed 2 years ago by rws

  • Commit changed from fa449e4ef165fdb4357a78d8f053c95a77ac6f0b to 7f69a644107405f1ade9b74d50118a04afac8826
  • Status changed from needs_work to needs_review

New commits:

7f69a6423199: missing sig_on/off in symbolic arithmetics

comment:8 Changed 2 years ago by jdemeyer

  • Status changed from needs_review to needs_work

This is wrong:

try:
    sig_on()
    [...]
finally:
    sig_off()

It should be

sig_on()
try:
    [...]
finally:
    sig_off()

comment:9 Changed 2 years ago by git

  • Commit changed from 7f69a644107405f1ade9b74d50118a04afac8826 to af5ef8bcee04b138ca4d3dce279e8149fbb4e999

Branch pushed to git repo; I updated commit sha1. New commits:

af5ef8b23199: fix previous commit

comment:10 Changed 2 years ago by rws

  • Status changed from needs_work to needs_review

comment:11 Changed 2 years ago by tscrim

  • Reviewers set to Jeroen Demeyer, Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:12 Changed 2 years ago by jdemeyer

+1

comment:13 Changed 2 years ago by vbraun

  • Branch changed from u/rws/23199-1 to af5ef8bcee04b138ca4d3dce279e8149fbb4e999
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:14 follow-up: Changed 2 years ago by thansen

  • Commit af5ef8bcee04b138ca4d3dce279e8149fbb4e999 deleted

With sage 8.0.beta11 building against Debian packages we still get this segfault after the docbuild. We are already using pynac 0.7.8.

Done building the documentation!
------------------------------------------------------------------------
/usr/lib/python2.7/dist-packages/cysignals/signals.x86_64-linux-gnu.so(+0x4e77)[0x7fb1c8788e77]
/usr/lib/python2.7/dist-packages/cysignals/signals.x86_64-linux-gnu.so(+0x5a05)[0x7fb1c8789a05]
/usr/lib/python2.7/dist-packages/cysignals/signals.x86_64-linux-gnu.so(+0x8057)[0x7fb1c878c057]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x110c0)[0x7fb1cdf3e0c0]
python(PyErr_Fetch+0xa)[0x55fbc80988da]
/home/thansen/src/sage/sagemath/sagemath/debian/build/usr/lib/python2.7/dist-packages/sage/rings/number_field/number_field_element_quadratic.so(+0x8a0f)[0x7fb0b5a36a0f]
/usr/lib/x86_64-linux-gnu/libpynac.so.13(+0x169451)[0x7fb09bb3f451]
/lib/x86_64-linux-gnu/libc.so.6(+0x35910)[0x7fb1cd29e910]
/lib/x86_64-linux-gnu/libc.so.6(+0x3596a)[0x7fb1cd29e96a]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf8)[0x7fb1cd2892b8]
python(_start+0x2a)[0x55fbc806031a]
------------------------------------------------------------------------
Attaching gdb to process id 6055.

Saved trace to /home/thansen/src/sage/sagemath/sagemath/debian/test/crash_logs/crash_tq1JYo.log
------------------------------------------------------------------------
Unhandled SIGSEGV: A segmentation fault occurred.
This probably occurred because a *compiled* module has a bug
in it and is not properly wrapped with sig_on(), sig_off().
Python will now terminate.

The crash log file is empty.

comment:15 in reply to: ↑ 14 Changed 2 years ago by rws

Moved to #23264

comment:16 Changed 10 months ago by jdemeyer

I have some doubts about this ticket now. The problem is that Pynac is executing Python code and executing Python code during sig_on() is not supported. This is especially problematic in case that garbage collection is done during sig_on(). See #27060

comment:17 Changed 10 months ago by jdemeyer

Ralf, do you remember exactly which problem this ticket was supposed to address? When removing those sig_on() statements again in #27060, I don't see any problems appearing. So either the problem here is fixed in a different way or we are missing tests for it.

Note: See TracTickets for help on using tickets.