Ticket #510 (closed enhancement: fixed)

Opened 1 year ago

Last modified 1 month ago

[with patch, positive review] Make sure importing sage.rings.real_mpfr without an "from sage import *" doesn't segfault Sage

Reported by: burcin Assigned to: was
Priority: major Milestone: sage-3.2
Component: misc Keywords:
Cc:

Description

Importing the module sage.rings.real_mpfr in IPython causes a segfault with the backtrace included below. SAGE itself doesn't report any errors in this case. 

In [2]: import sage.rings.real_mpfr

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1210221920 (LWP 28996)]
0xb0d29b0e in __pyx_f_9real_mpfr_9RealField___init__ (__pyx_v_self=0xb0dc625c, 
    __pyx_args=0xb0ce242c, __pyx_kwds=0x0) at sage/rings/real_mpfr.c:1128
1128      Py_INCREF(__pyx_v_rnd);
(gdb) bt
#0  0xb0d29b0e in __pyx_f_9real_mpfr_9RealField___init__ (
    __pyx_v_self=0xb0dc625c, __pyx_args=0xb0ce242c, __pyx_kwds=0x0)
    at sage/rings/real_mpfr.c:1128
#1  0x0809c2f3 in type_call (type=0xb0d88d40, args=0xb0ce242c, kwds=0x0)
    at Objects/typeobject.c:436
#2  0x0805a0a7 in PyObject_Call (func=0x0, arg=0xb0ce242c, kw=0x0)
    at Objects/abstract.c:1860
#3  0x080c0c50 in PyEval_EvalFrameEx (f=0x83964ec, throwflag=0)
    at Python/ceval.c:3775
#4  0x080c564a in PyEval_EvalFrameEx (f=0x8392a9c, throwflag=0)
    at Python/ceval.c:3650
#5  0x080c639b in PyEval_EvalCodeEx (co=0xb0dd0ad0, globals=0xb0d9fc64, 
    locals=0x0, args=0xb0d963f8, argcount=2, kws=0x0, kwcount=0, 
    defs=0xb0ce2478, defcount=1, closure=0x0) at Python/ceval.c:2831

You can use the patch included in #509 to run IPython under gdb.

Attachments

510_real_mpfr_imports.patch (5.3 kB) - added by burcin on 05/10/2008 12:17:47 PM.
fix some circular imports in sage.rings.real_mpfr
sage-test-import (1.5 kB) - added by burcin on 10/15/2008 09:44:30 AM.
script to test if importing a random module from the sage library returns an error

Change History

08/31/2007 01:09:10 PM changed by was

  • type changed from defect to enhancement.

This is not so much a bug as something that hasn't been implemented -- or even thought about yet. It doesn't make sense to just load any random module into SAGE, since certain modules assume certain other modules have been imported first. E.g., import sage.math first and the rest works fine: 

In [1]: import sage.all

In [2]: import sage.rings.real_mpfr

In short, importing particular random modules without having import sage.all first is *not* supported at present. It would be a good enhancement to implement something to force sage.all to be imported or something...

09/06/2007 10:29:48 PM changed by craigcitro

  • component changed from algebraic geometry to interfaces.

02/17/2008 11:48:18 AM changed by mabshoff

  • summary changed from IPython crashes on "import sage.rings.real_mpfr" to [probably invalid] IPython crashes on "import sage.rings.real_mpfr".

I would consider this bug report to be invalid - see the explanation by was above, so it should be closed as invalid. Any opinioins?

Cheers,

Michael

02/17/2008 01:02:32 PM changed by mabshoff

  • status changed from new to closed.
  • resolution set to invalid.
  • milestone changed from sage-2.10.2 to sage-duplicate/invalid.
[21:42] <wstein> mabshoff -- close 510 as invalid.
[21:42] <mabshoff> ok

02/18/2008 12:48:33 AM changed by burcin

I think this should still remain as an enhancement request, maybe in the sage-wishlist target.

The reason we get a segfault by importing sage.rings.real_mpfr before sage.all is that sage.all calls some initialization function for this library. From a software design point of view, this initialization should be handled closer to the place the library is used, possibly in the __init__.py of the corresponding wrapper. Then users will be able to import this module without worrying about importing sage.all and waiting to initialize many components they don't care about.

Even the fact that we don't know which initialization function needs to be called to fix the segfault is enough to regard this as a bug in the wrapper. Since nobody cares enough to look into this at the moment, we should at least keep note of it as an enhancement request which we could take up, once we have the time.

So I suggest we reopen this, and maybe resolve to wontfix instead of invalid next time.

02/18/2008 05:14:46 AM changed by mabshoff

  • status changed from closed to reopened.
  • summary changed from [probably invalid] IPython crashes on "import sage.rings.real_mpfr" to Make sure importing random modules from Sage doesn't segfault.
  • resolution deleted.
  • milestone changed from sage-duplicate/invalid to sage-wishlist.

ok, I am convinced for now. If you can come up with a better summary for the bug please change it.

Cheers,

Michael

05/10/2008 12:17:47 PM changed by burcin

  • attachment 510_real_mpfr_imports.patch added.

fix some circular imports in sage.rings.real_mpfr

05/10/2008 12:25:19 PM changed by burcin

attachment:510_real_mpfr_imports.patch fixes the problem reported in the original report. With this patch, 

import sage.rings.complex_field

still fails because of similar errors. I'll write a script which imports each file in the tree individually and see if I can fix the errors.

09/30/2008 04:49:00 AM changed by mabshoff

  • summary changed from Make sure importing random modules from Sage doesn't segfault to [with patch, needs review] Make sure importing random modules from Sage doesn't segfault.

This ticket should be renamed and the patch that currently exists should be reviewed and hopefully merged. Since the patch is a little old it might have gone stale, but hopefully not.

If a test script exists we should definitely fix all the problems that it detects, but those problems should be handled via new tickets.

Cheers,

Michael

09/30/2008 05:22:29 AM changed by mabshoff

  • summary changed from [with patch, needs review] Make sure importing random modules from Sage doesn't segfault to [with patch, needs review] Make sure importing sage.rings.real_mpfr without an "from sage import *" doesn't segfault Sage.
  • milestone changed from sage-wishlist to sage-3.2.

Amazingly this patch still applies with some fuzz to my current 3.1.3.alpha2 merge tree: 

age-3.1.3.alpha2/devel/sage$ patch -p1 --dry-run < 510_real_mpfr_imports.patch\?format\=raw 
patching file sage/rings/real_double.pxd
patching file sage/rings/real_double.pyx
Hunk #2 succeeded at 1088 (offset 9 lines).
patching file sage/rings/real_mpfr.pyx
Hunk #1 succeeded at 132 (offset 6 lines).
Hunk #2 succeeded at 158 (offset 6 lines).
Hunk #3 succeeded at 237 with fuzz 2 (offset 18 lines).
Hunk #4 succeeded at 468 (offset 63 lines).
Hunk #5 succeeded at 1879 (offset 191 lines).
patching file sage/rings/real_rqdf.pxd
patching file sage/rings/real_rqdf.pyx
Hunk #3 succeeded at 1263 (offset 4 lines).

I did not try to compile, but I consider this encouraging.

10/15/2008 09:44:30 AM changed by burcin

  • attachment sage-test-import added.

script to test if importing a random module from the sage library returns an error

10/15/2008 09:48:26 AM changed by burcin

  • component changed from interfaces to misc.

attachment:sage-test-import imports all modules in the given source tree, and prints out those which raise an error.

10/15/2008 10:09:45 AM changed by burcin

I just tried attachment:510_real_mpfr_imports.patch. It still applies, and fixes the original problem reported, as I wrote in comment:7 previously.

The script attachment:sage-test-import lists many other problems of this kind, but feel free to apply this patch, and move the script to another ticket.

10/26/2008 08:38:09 PM changed by mabshoff

  • summary changed from [with patch, needs review] Make sure importing sage.rings.real_mpfr without an "from sage import *" doesn't segfault Sage to [with patch, positive review] Make sure importing sage.rings.real_mpfr without an "from sage import *" doesn't segfault Sage.

Positive review on the patch. I also added the sage-test-import script to the local/bin repo.

Cheers,

Michael

10/26/2008 08:40:31 PM changed by mabshoff

  • status changed from reopened to closed.
  • resolution set to fixed.
  • milestone changed from sage-3.2.1 to sage-3.2.

Merged in Sage 3.2.alpha1