Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
qed777 mannequin opened this issue Aug 12, 2010 · 10 comments
Closed

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

qed777 mannequin opened this issue Aug 12, 2010 · 10 comments

Comments

@qed777
Copy link
Mannequin

qed777 mannequin commented Aug 12, 2010

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.

CC: @robertwb @williamstein @jdemeyer

Component: doctest coverage

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

@qed777 qed777 mannequin added this to the sage-4.5.3 milestone Aug 12, 2010
@qed777 qed777 mannequin added t: tests labels Aug 12, 2010
@qed777 qed777 mannequin assigned sagetrac-mvngu Aug 12, 2010
@qed777
Copy link
Mannequin Author

qed777 mannequin commented Aug 12, 2010

comment:1

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

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Aug 13, 2010

Attachment: genus2reduction.c.diff.gz

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

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Aug 13, 2010

comment:2

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

@jdemeyer
Copy link

comment:4

Replying to @qed777:

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).

@jdemeyer
Copy link

Attachment: 9738_genus2reduction_init_opts.patch.gz

Alternative patch to fix the issue

@jdemeyer
Copy link

comment:5

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.

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Aug 13, 2010

comment:6

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.

@qed777 qed777 mannequin added p: major / 3 and removed p: blocker / 1 labels Aug 14, 2010
@jdemeyer
Copy link

comment:8

Replying to @qed777:

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.

@jdemeyer
Copy link

comment:9

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

@jdemeyer jdemeyer removed this from the sage-4.5.3 milestone Aug 14, 2010
@qed777
Copy link
Mannequin Author

qed777 mannequin commented Aug 31, 2010

comment:10

Release manager

Please close this ticket when #9591 is merged.

@qed777 qed777 mannequin added s: positive review and removed s: needs review labels Sep 4, 2010
@qed777 qed777 mannequin closed this as completed Sep 10, 2010
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant