segfault on Graph().connected_component_containing_vertex('') #11942

kini · 2011-10-21T05:55:24Z

As title.

fs@zhenghe /tmp $ sage -c "Graph().connected_component_containing_vertex('')"
/opt/sage/local/lib/libcsage.so(print_backtrace+0x31)[0x7f14e1a96df2]
/opt/sage/local/lib/libcsage.so(sigdie+0x14)[0x7f14e1a96e24]
/opt/sage/local/lib/libcsage.so(sage_signal_handler+0x20c)[0x7f14e1a96a72]
/lib/x86_64-linux-gnu/libpthread.so.0(+0xf020)[0x7f14e31fa020]
/opt/sage/local/lib/python2.6/site-packages/sage/graphs/base/c_graph.so(+0x11aa1)[0x7f14be4d8aa1]
/opt/sage/local/lib/libpython2.6.so.1.0(+0x75876)[0x7f14e347c876]
/opt/sage/local/lib/libpython2.6.so.1.0(+0x75b92)[0x7f14e347cb92]
/opt/sage/local/lib/libpython2.6.so.1.0(+0xa0f08)[0x7f14e34a7f08]
/opt/sage/local/lib/libpython2.6.so.1.0(PyObject_Call+0x53)[0x7f14e3451cd3]
/opt/sage/local/lib/libpython2.6.so.1.0(PyEval_EvalFrameEx+0x395d)[0x7f14e34e835d]
/opt/sage/local/lib/libpython2.6.so.1.0(PyEval_EvalFrameEx+0x602d)[0x7f14e34eaa2d]
/opt/sage/local/lib/libpython2.6.so.1.0(PyEval_EvalCodeEx+0x879)[0x7f14e34eb549]
/opt/sage/local/lib/libpython2.6.so.1.0(PyEval_EvalCode+0x32)[0x7f14e34eb642]
/opt/sage/local/lib/libpython2.6.so.1.0(+0xdb07e)[0x7f14e34e207e]
/opt/sage/local/lib/libpython2.6.so.1.0(PyEval_EvalFrameEx+0x4f8d)[0x7f14e34e998d]
/opt/sage/local/lib/libpython2.6.so.1.0(PyEval_EvalCodeEx+0x879)[0x7f14e34eb549]
/opt/sage/local/lib/libpython2.6.so.1.0(PyEval_EvalCode+0x32)[0x7f14e34eb642]
/opt/sage/local/lib/libpython2.6.so.1.0(PyRun_FileExFlags+0xb0)[0x7f14e350d9b0]
/opt/sage/local/lib/libpython2.6.so.1.0(PyRun_SimpleFileExFlags+0xdf)[0x7f14e350e3ef]
/opt/sage/local/lib/libpython2.6.so.1.0(Py_Main+0xb23)[0x7f14e351b693]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xfd)[0x7f14e27fcead]
python[0x4006a1]

------------------------------------------------------------------------
Unhandled SIGSEGV: A segmentation fault occurred in Sage.
This probably occurred because a *compiled* component of Sage has a bug
in it and is not properly wrapped with sig_on(), sig_off(). You might
want to run Sage under gdb with 'sage -gdb' to debug this.
Sage will now terminate.
------------------------------------------------------------------------
/opt/sage/local/bin/sage-sage: line 772: 19065 Segmentation fault      sage-eval "$@"
fs@zhenghe /tmp $

Apply:

Component: graph theory

Keywords: segfault cython

Author: Nathann Cohen, Keshav Kini, Jeroen Demeyer

Reviewer: Keshav Kini, David Coudert, Nathann Cohen

Merged: sage-5.0.beta0

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

The text was updated successfully, but these errors were encountered:

nathanncohen · 2011-10-21T15:29:05Z

Author: Nathann Cohen

nathanncohen · 2011-10-21T15:29:05Z

comment:1

Right ! :-p

This comes from an unchecked operation at the beginning of BFS/DFS. Fixed in the following patch :-)

Nathann

kini · 2011-10-22T07:31:19Z

Reviewer: Keshav Kini

kini · 2011-10-22T07:31:19Z

comment:2

Wow, that was fast! :D I have a couple of questions, though. Graph().connected_component_containing_vertex(0) already produces an exception, which includes the number 0 in the message. This is still true with your patch. Currently investigating why...

Also, your exception is a ValueError - shouldn't it be a KeyError? Shouldn't the current error for vertex 0 be a KeyError too instead of a (!?) RuntimeError? The Python docs say:

This exception is mostly a relic from a previous version of the interpreter; it is not used very much any more.

nathanncohen · 2011-10-27T17:57:11Z

comment:3

Hellooooooo !

This exception is mostly a relic from a previous version of the interpreter; it is not used very much any more.

Oh ? That's a pity, I really use it a lot :-)

I immediately update the patch to return a KeyError :-)

Nathann

kini · 2011-10-27T17:59:14Z

comment:4

Oh, ValueError is still used a lot but it is RuntimeError which is not used much anymore.

nathanncohen · 2011-10-27T18:01:37Z

comment:5

Oh, ValueError is still used a lot but it is RuntimeError which is not used much anymore.

Argggggg !!!

Patch reverted to the former one :-D

Nathann

kini · 2011-10-27T18:08:01Z

comment:6

Attachment: trac_11942.patch.gz

... but... even though ValueError is used a lot, the correct one to use here is KeyError... D:

Anyway don't worry about it, I'll do that in another patch later :) Sorry, didn't get much time to look at this earlier this week.

nathanncohen · 2011-10-27T18:09:27Z

comment:7

Replying to @kini:

... but... even though ValueError is used a lot, the correct one to use here is KeyError... D:

Arggggggggggg !!!

Anyway don't worry about it, I'll do that in another patch later :) Sorry, didn't get much time to look at this earlier this week.

Good !! THaaaaanks ! :-)

Nathann

jdemeyer · 2011-11-03T16:14:43Z

Milestone sage-4.7.3 deleted

dcoudert · 2011-12-10T20:06:01Z

Changed reviewer from Keshav Kini to Keshav Kini, David Coudert

dcoudert · 2011-12-10T20:06:01Z

comment:9

I don't understand why you have both stopped the discussion without concluding on the status of this patch.

I have no problem when installing the patch and building the documentation.
The function tests correctly that the parameter is or is not a vertex of the graph.

For me the patch is correct, so I give a positive review.

Best,
David.

kini · 2011-12-10T20:21:47Z

comment:10

Please read the comments. The ValueError needs to be changed to KeyError, and also I wanted to understand the following before giving it a positive review, as possibly the fix could be improved (multiple code paths for a single purpose is bad design):

Graph().connected_component_containing_vertex(0) already produces an exception, which includes the number 0 in the message. This is still true with your patch.

Sorry for neglecting this ticket for so long.

kini · 2012-01-04T01:50:20Z

Changed keywords from none to segfault cython

kini · 2012-01-04T01:50:20Z

comment:11

Well, I'm still not sure about the multiple code paths. But here is a patch to use better exceptions throughout the file in question, though that's sort of tangential to the original purpose of the ticket. This ticket has been sitting around too long for me to delay it any further just because of my ignorance of the Cython graphs code, especially since it fixes a segfault, so I will give a positive review to Nathann's part. Maybe someone can review my exceptions patch? It passes make ptestlong on my machine fwiw.

nathanncohen · 2012-01-05T13:36:47Z

Changed reviewer from Keshav Kini, David Coudert to Keshav Kini, David Coudert, Nathann Cohen

kini · 2012-01-05T14:04:24Z

comment:20

Hurrah! Thanks for the review (and the original patch)!

BTW, I botched that 2001 quote above... it should be "I'm sorry, Dave, I'm afraid I can't do that", of course :P

nathanncohen · 2012-01-05T14:07:01Z

comment:21

BTW, I botched that 2001 quote above... it should be "I'm sorry, Dave, I'm afraid I can't do that", of course :P

No harm done, we both watched the french version of it :-D

jdemeyer · 2012-01-05T14:36:59Z

Use LookupError instead of ValueError or KeyError

jdemeyer · 2012-01-05T14:38:11Z

Changed author from Nathann Cohen, Keshav Kini to Nathann Cohen, Keshav Kini, Jeroen Demeyer

jdemeyer · 2012-01-05T14:38:11Z

comment:23

Attachment: trac_11942-exceptions.2.patch.gz

Since KeyError is specifically about dicts, I think the more general LookupError is better. Needs review.

kini · 2012-01-05T14:58:40Z

comment:25

Oh, I was under the impression that those base classes shouldn't be instantiated, but that seems to be unfounded after reading the docs. Looks good to me! Lines 855-856 of $SAGE_ROOT/devel/sage/sage/graphs/base/graph_backends.py contains another exception which seems like it should be a LookupError but I'm not sure I understand why the current exception is imported from !NetworkX. Maybe there is a good reason for it?

kini · 2012-01-05T14:58:57Z

comment:26

Whoops, didn't mean to change the milestone.

nathanncohen · 2012-01-05T15:04:06Z

comment:27

Well, these lines are contained in the NetworkX backend, so it may mean that they define their own Exceptions in the NetworkX project. I do not think this class is still used in Sage anyway.

Nathann

kini · 2012-01-05T15:06:19Z

comment:28

But didn't Sage developers write that NetworkX backend wrapper? Well, never mind, if it's not used in Sage anymore.

nathanncohen · 2012-01-05T15:11:39Z

comment:29

Well yes, and that's most probably this class. But if you run some code made for NetworkX on a Sage graph with our own exception it may break the NetworkX code. I guess this was the point of importing exceptions. Thank you for the patch Jeroen !

Nathann

kini · 2012-01-05T15:36:03Z

comment:30

Replying to @nathanncohen:

if you run some code made for NetworkX on a Sage graph

Aha, now I get it. And yes, thanks Jeroen :)

jdemeyer · 2012-01-15T21:56:17Z

Merged: sage-5.0.beta0

kini added this to the sage-4.8 milestone Oct 21, 2011

kini added c: graph theory labels Oct 21, 2011

kini assigned jasongrout, nathanncohen and rlmill Oct 21, 2011

This comment has been minimized.

Sign in to view

nathanncohen mannequin added the s: needs review label Oct 21, 2011

jdemeyer removed this from the sage-4.8 milestone Nov 3, 2011

dcoudert added s: positive review and removed s: needs review labels Dec 10, 2011

dcoudert added this to the sage-4.8 milestone Dec 10, 2011

kini added s: needs work and removed s: positive review labels Dec 10, 2011

nathanncohen mannequin added s: positive review and removed s: needs review labels Jan 5, 2012

jdemeyer modified the milestones: sage-4.8, sage-5.0 Jan 5, 2012

This comment has been minimized.

Sign in to view

jdemeyer added s: needs work and removed s: positive review labels Jan 5, 2012

kini modified the milestones: sage-5.0, sage-4.8 Jan 5, 2012

kini modified the milestones: sage-4.8, sage-5.0 Jan 5, 2012

kini added s: positive review and removed s: needs review labels Jan 5, 2012

jdemeyer removed the s: positive review label Jan 15, 2012

jdemeyer closed this as completed Jan 15, 2012

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

segfault on Graph().connected_component_containing_vertex('') #11942

segfault on Graph().connected_component_containing_vertex('') #11942

kini commented Oct 21, 2011

This comment has been minimized.

nathanncohen mannequin commented Oct 21, 2011

nathanncohen mannequin commented Oct 21, 2011

kini commented Oct 22, 2011

kini commented Oct 22, 2011

nathanncohen mannequin commented Oct 27, 2011

kini commented Oct 27, 2011

nathanncohen mannequin commented Oct 27, 2011

kini commented Oct 27, 2011

nathanncohen mannequin commented Oct 27, 2011

jdemeyer commented Nov 3, 2011

dcoudert commented Dec 10, 2011

dcoudert commented Dec 10, 2011

kini commented Dec 10, 2011

kini commented Jan 4, 2012

kini commented Jan 4, 2012

nathanncohen mannequin commented Jan 5, 2012

kini commented Jan 5, 2012

nathanncohen mannequin commented Jan 5, 2012

jdemeyer commented Jan 5, 2012

jdemeyer commented Jan 5, 2012

jdemeyer commented Jan 5, 2012

This comment has been minimized.

kini commented Jan 5, 2012

kini commented Jan 5, 2012

nathanncohen mannequin commented Jan 5, 2012

kini commented Jan 5, 2012

nathanncohen mannequin commented Jan 5, 2012

kini commented Jan 5, 2012

jdemeyer commented Jan 15, 2012

segfault on Graph().connected_component_containing_vertex('') #11942

segfault on Graph().connected_component_containing_vertex('') #11942

Comments

kini commented Oct 21, 2011

This comment has been minimized.

nathanncohen mannequin commented Oct 21, 2011

nathanncohen mannequin commented Oct 21, 2011

kini commented Oct 22, 2011

kini commented Oct 22, 2011

nathanncohen mannequin commented Oct 27, 2011

kini commented Oct 27, 2011

nathanncohen mannequin commented Oct 27, 2011

kini commented Oct 27, 2011

nathanncohen mannequin commented Oct 27, 2011

jdemeyer commented Nov 3, 2011

dcoudert commented Dec 10, 2011

dcoudert commented Dec 10, 2011

kini commented Dec 10, 2011

kini commented Jan 4, 2012

kini commented Jan 4, 2012

nathanncohen mannequin commented Jan 5, 2012

kini commented Jan 5, 2012

nathanncohen mannequin commented Jan 5, 2012

jdemeyer commented Jan 5, 2012

jdemeyer commented Jan 5, 2012

jdemeyer commented Jan 5, 2012

This comment has been minimized.

kini commented Jan 5, 2012

kini commented Jan 5, 2012

nathanncohen mannequin commented Jan 5, 2012

kini commented Jan 5, 2012

nathanncohen mannequin commented Jan 5, 2012

kini commented Jan 5, 2012

jdemeyer commented Jan 15, 2012