Opened 10 years ago

Closed 10 years ago

#9738 closed defect (duplicate)

Stealth core dump from testing sage/interfaces/genus2reduction.py

Reported by: mpatel Owned by: mvngu
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: doctest coverage Keywords:
Cc: robertwb, was, jdemeyer Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

With Sage 4.5.3.alpha0 on sage.math:

$ cd SAGE_ROOT
$ find -name core -type f
$ ulimit -c unlimited
$ ./sage -t -long devel/sage/sage/interfaces/genus2reduction.py 
sage -t -long "devel/sage/sage/interfaces/genus2reduction.py"
         [3.1 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 3.1 seconds
$ find -name core -type f
./data/extcode/genus2reduction/core
$

For background see sage-devel.

Attachments (2)

genus2reduction.c.diff (763 bytes) - added by mpatel 10 years ago.
Quit on quit. Diff of genus2reduction.c from g2r 0.3.p6.
9738_genus2reduction_init_opts.patch (2.3 KB) - added by jdemeyer 10 years ago.
Alternative patch to fix the issue

Download all attachments as: .zip

Change History (14)

comment:1 Changed 10 years ago by mpatel

I interleaved find -name core -type f with executing the examples in genus2reduction.py in an interactive Sage session. I see the core file only after I exit Sage, which ends with

Exiting Sage (CPU time 0m0.17s, Wall time 2m6.89s).
Exiting spawned Genus2reduction process.

Also, here is the log I find in DOT_SAGE/pexpect_logs/ after

env SAGE_PEXPECT_LOG="yes" ./sage -t devel/sage/sage/interfaces/genus2reduction.py

Changed 10 years ago by mpatel

Quit on quit. Diff of genus2reduction.c from g2r 0.3.p6.

comment:2 follow-up: Changed 10 years ago by mpatel

  • Cc was added

I've attached a possible solution. If it looks good, I can make a new spkg. We may need to coordinate with #9591.

comment:3 Changed 10 years ago by jdemeyer

  • Cc jdemeyer added

comment:4 in reply to: ↑ 2 Changed 10 years ago by jdemeyer

Replying to mpatel:

We may need to coordinate with #9591.

I don't think there is a problem, this issue is independent of #9343 and #9591 (we just have to rename the spkg at #9591 if this gets merged first).

Changed 10 years ago by jdemeyer

Alternative patch to fix the issue

comment:5 Changed 10 years ago by jdemeyer

I think mpatel's patch fixes the problem but does not address the real issue here, namely that PARI by default catches various signals and "handles" them. But this is not what we want here. Not making PARI install signal handlers solves the issue.

My patch also makes genus2reduction exit when EOF is encountered in the standard input.

comment:6 follow-up: Changed 10 years ago by mpatel

Jeroen's patch is definitely better than mine. Thanks!

Whether I run the doctest above or run genus2reduction directly from the shell and press ctrl-c/d, the program quits with no core dump. Evaluating genus2reduction.console() in the Sage console and pressing ctrl-c/d returns me to the sage: prompt. Also, running the long doctest suite passes without reproducible failures and leaves no relevant cores.

Genus2reduction_expect in genus2reduction.py still uses its base class' Expect._quit_string, which returns "quit", but I think we can leave that alone(?).

Are there any objections to making a new spkg here with Jeroen's patch? If we do, we should put genus2reduction.c under version control. Although I'm averse to putting too many logically different spkg changes in one ticket, I'll understand if we roll the changes here into #9591 and make this ticket a virtual blocker for an otherwise PARI-focused Sage 4.6.

comment:7 Changed 10 years ago by mpatel

  • Priority changed from blocker to major

comment:8 in reply to: ↑ 6 Changed 10 years ago by jdemeyer

Replying to mpatel:

Are there any objections to making a new spkg here with Jeroen's patch?

No objections. If you think this patch can make it into sage-4.5.3, go ahead and do it. If we merge it in sage-4.6, it would be better suited in #9591.

comment:9 Changed 10 years ago by jdemeyer

  • Milestone changed from sage-4.5.3 to sage-duplicate/invalid/wontfix
  • Status changed from new to needs_review

Since there seems to be a push towards merging #9343 (and hence #9591) soon, I will merge my patch into #9591.

comment:10 Changed 10 years ago by mpatel

Release manager

Please close this ticket when #9591 is merged.

comment:11 Changed 10 years ago by mpatel

  • Status changed from needs_review to positive_review

comment:12 Changed 10 years ago by mpatel

  • Resolution set to duplicate
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.