sagemath/sage

segmentation fault in pexpect for singular

fchapoton opened this issue · 30 comments

At least two patchbots, one Linux and one macOS, meet a failure
related to pexpect interface for Singular, namely:

sage -t --long --random-seed=0 src/sage/interfaces/singular.py  # Killed due to segmentation fault

The tested file also fails when run alone, but the copy-pasted test works fine.

See https://groups.google.com/d/msgid/sage-devel/54c8dd1f-5efa-4a9f-b17f-4225ef9c6c91n%40googlegroups.com.

The following test fails:

sage: try:
    alarm(0.5)
    singular._expect_expr('>')  # interrupt this
except KeyboardInterrupt:
    pass ## line 505 ##
Control-C pressed. Interrupting Singular. Please wait a few seconds...
sage: 2*a ## line 514 ##
------------------------------------------------------------------------
/home/chapoton/sage/local/lib/python3.8/site-packages/cysignals/signals.cpython-38-x86_64-linux-gnu.so(+0x7df4)[0x7f7c33af8df4]
...

For full logs, see:

CC: @dimpase @slel @sagetrac-tmonteil @kiwifb

Component: interfaces

Keywords: singular, segfault

Reviewer: Michael Orlitzky

Issue created by migration from https://trac.sagemath.org/ticket/30945

comment:1

changing alarm(0.5) to either alarm(0.5*256) or alarm(0.5/256) does not fix the issue

slel commented
comment:3

I have sometimes seen that failure too.

slel commented

Description changed:

--- 
+++ 
@@ -1,14 +1,13 @@
-At least two patchbots, one Linux and one MacOs, meet a failure related to pexpect interface for singular, namely:
+At least two patchbots, one Linux and one macOS, meet a failure
+related to pexpect interface for Singular, namely:
 
+```
 sage -t --long --random-seed=0 src/sage/interfaces/singular.py  # Killed due to segmentation fault
-
+```
 The tested file also fails when run alone, but the copy-pasted test works fine.
 
-Frédéric
+For full logs, see:
 
-PS : for full logs, see
+- [steenrod report](https://patchbot.sagemath.org/log/0/Darwin/Darwin%20Kernel%20Version%2018.5.0:%20Mon%20Mar%2011%2020:40:32%20PDT%202019;%20root:xnu-4903.251.3~3/RELEASE_X86_64/x86_64/18.5.0/steenrod/2020-11-07%2019:50:32?short)
+- [petitbonum report](https://patchbot.sagemath.org/log/0/Linux/57-Ubuntu_SMP_Thu_Oct_15_10:57:00_UTC_2020/x86_64/5.4.0-52-generic/petitbonum/2020-11-07%2020:48:10?short)
 
-https://patchbot.sagemath.org/log/0/Darwin/Darwin%20Kernel%20Version%2018.5.0:%20Mon%20Mar%2011%2020:40:32%20PDT%202019;%20root:xnu-4903.251.3~3/RELEASE_X86_64/x86_64/18.5.0/steenrod/2020-11-07%2019:50:32?short
-
-https://patchbot.sagemath.org/log/0/Linux/57-Ubuntu_SMP_Thu_Oct_15_10:57:00_UTC_2020/x86_64/5.4.0-52-generic/petitbonum/2020-11-07%2020:48:10?short
-
slel commented

Changed keywords from none to singular, segfault

comment:4

Je suis preneur de toute suggestion.

Description changed:

--- 
+++ 
@@ -6,6 +6,23 @@
 ```
 The tested file also fails when run alone, but the copy-pasted test works fine.
 
+See https://groups.google.com/d/msgid/sage-devel/54c8dd1f-5efa-4a9f-b17f-4225ef9c6c91n%40googlegroups.com.
+
+The following test fails:
+
+```
+sage: try:
+    alarm(0.5)
+    singular._expect_expr('>')  # interrupt this
+except KeyboardInterrupt:
+    pass ## line 505 ##
+Control-C pressed. Interrupting Singular. Please wait a few seconds...
+sage: 2*a ## line 514 ##
+------------------------------------------------------------------------
+/home/chapoton/sage/local/lib/python3.8/site-packages/cysignals/signals.cpython-38-x86_64-linux-gnu.so(+0x7df4)[0x7f7c33af8df4]
+...
+```
+
 For full logs, see:
 
 - [steenrod report](https://patchbot.sagemath.org/log/0/Darwin/Darwin%20Kernel%20Version%2018.5.0:%20Mon%20Mar%2011%2020:40:32%20PDT%202019;%20root:xnu-4903.251.3~3/RELEASE_X86_64/x86_64/18.5.0/steenrod/2020-11-07%2019:50:32?short)
comment:6

Some date points, in case it might be useful: on my Ubuntu 20.04 computer, running sage -t --long src/sage/interfaces/singular.py

  • triggers the segfault with Sage 9.3.beta2 and 9.2
  • returns "All tests passed!" with Sage 9.1
comment:7

pexpect was last updated in #29240, included in Sage 9.2.beta11

comment:8

Perhaps this test failure has nothing to do Singular.

singular's is the only pexpect interface on which this test is carried out. One could also have same for GAP, say:

sage: a = gap(1)
sage: _ = gap._expect.sendline('1+')  # unfinished input
sage: try:
....:     alarm(0.5)
....:     gap._expect_expr('gap>')  # interrupt this
....: except KeyboardInterrupt:
....:     pass                                                                                                                                                                                
Control-C pressed. Interrupting Gap. Please wait a few seconds...
sage: 2*a                                                                                                                                                                      
2

Could someone who has a box where this is reliably reproduced, try adding somewhere the doctest for GAP above, and see if it fails too?

comment:9

Bug fails with gap too, when running doctests. In the command line, provokes a very bad crash of sage.

running in the doctests:

File "src/sage/interfaces/singular.py", line 511, in sage.interfaces.singular.Singular._send_interrupt
Failed example:
    2*a
Exception raised:
    Traceback (most recent call last):
      File "/home/chapoton/sage/local/lib/python3.8/site-packages/sage/interfaces/gap.py", line 763, in _eval_line
        raise RuntimeError("%s produced error output\n%s\n   executing %s"%(self, error,line))
    RuntimeError: Gap produced error output
    Error, no method found! For debugging hints type ?Recovery from NoMethodFound
    Error, no 1st choice method found for `*' on 2 arguments

       executing \$sage3:=\$sage2 * \$sage1;;

    During handling of the above exception, another exception occurred:

    Traceback (most recent call last):
      File "/home/chapoton/sage/local/lib/python3.8/site-packages/sage/interfaces/expect.py", line 1469, in __init__
        self._name = parent._create(value, name=name)
      File "/home/chapoton/sage/local/lib/python3.8/site-packages/sage/interfaces/interface.py", line 501, in _create
        self.set(name, value)
      File "/home/chapoton/sage/local/lib/python3.8/site-packages/sage/interfaces/gap.py", line 1422, in set
        self._eval_line(cmd, allow_use_file=True)
      File "/home/chapoton/sage/local/lib/python3.8/site-packages/sage/interfaces/gap.py", line 797, in _eval_line
        raise RuntimeError(exc)
    RuntimeError: Gap produced error output
    Error, no method found! For debugging hints type ?Recovery from NoMethodFound
    Error, no 1st choice method found for `*' on 2 arguments

       executing \$sage3:=\$sage2 * \$sage1;;
etc

in the console, one gets

sage: a                                                                                                                                     
(invalid Gap object -- The gap session in which this object was defined is no longer running.)
sage: 2*a
big mess as above
comment:10

OK, actually I should have checked, for me such GAP pexpect test in Sage console also results in
(invalid Gap object -- The gap session in which this object was defined is no longer running.)

This makes me wonder why it's important to have such a test in Singular. Only Singular has a custom _send_interrupt; GAP
uses the default one, below, and you see it sends _quit_string; it kills the GAP session if it lands in the main loop, rather than in the break loop (doing quit; in GAP's break loop brings you to the main loop, so in this case it's OK).

    def _send_interrupt(self):
        """
        Send an interrupt to the application.  This is used internally
        by :meth:`interrupt`.

        First CTRL-C to stop the current command, then quit.
        """
        self._expect.sendline(chr(3))
        self._expect.sendline(self._quit_string())

Singular custom one was added along with the test in question, and no _quit_string() is sent (which is quit; for GAP, as
you can see by looking at the value of gap._quit_string()).
For Singular one has the following in _send_interrupt():

        sleep(0.1)

        E = self._expect
        E.sendline(chr(3))
        for i in range(5):
            try:
                E.expect_upto(self._prompt, timeout=1.0)
                return
            except Exception:
                pass
            E.sendline(";")

We can try just to erase Singular's _send_interrupt(), set its _quit_string() to proper value (it's quit rather than quit;,
as it should be) and be done with.

comment:11

here is a tentative, just removing the _send_interrupt method


New commits:

95e1efdremove custom _send_interrupt in singular pexpect interface

Commit: 95e1efd

comment:12

Did you mean that exit is the singular quit command ??

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

85f65bfremove custom _send_interrupt in singular pexpect interface

Changed commit from 95e1efd to 85f65bf

comment:14

hmm, this seems to have a side effect, see the latest patchbot report

comment:15

the side effect could be that hitting ^C during a computation involving a session with pexpect Singular is no longer only (probably) interrupted, but the whole session is killed.

I am not sure it is safe do interrupt Singular - note the warning at the end

^C// ** Interrupt at cmd:`load` in line:';return();'
abort after this command(a), abort immediately(r), print backtrace(b), continue(c) or quit Singular(q) ?// ** Interrupt at cmd:`load` in line:';return();'
abort after this command(a), abort immediately(r), print backtrace(b), continue(c) or quit Singular(q) ?r
** Warning: Singular should be restarted as soon as possible **

in a console Singular session.

comment:16

I encountered this in #31404 as well. I believe I have isolated it to a possible bug in cysignals: sagemath/cysignals#126

comment:17

I am able to reproduce this pretty reliably on the current develop branch with

./sage -t --file-iterations=2 -T 0 --verbose src/sage/interfaces/singular.py
comment:18

This now has a possible fix at sagemath/cysignals#127

The segfault is not caused by Singular (if it were this would crash pexpect, not cause a segfault in Sage). It's caused by cysignals itself.

Dependencies: #31474

slel commented

Changed commit from 85f65bf to none

slel commented

Changed branch from public/singular_pexpect to none

slel commented
comment:21

Likely solved by cysignals 1.10.3 upgrade at #31474,
merged in Sage 9.3.rc0, released 2021-03-24.

If we get no more reports on sage-release in a while,
this ticket should be closed.

Can the branch public/singular_pexpect improving
interfaces/singular.py go to a new ticket?

slel commented
comment:22

Branch improving the pexpect interface to Singular now at #31846.

slel commented

Changed dependencies from #31474 to none

Reviewer: Michael Orlitzky

comment:23

Addressed in #31474 and #31846.