msg302375 - (view) | Author: Stefan Behnel (scoder) *  | Date: 2017-09-17 17:27 |
I'm seeing crashes in the latest Py3.7 when I run this test (taken from lxml's compatibility test suite): etree = xml.etree.ElementTree def test_feed_parser_error_position(self): ParseError = etree.ParseError parser = XMLParser() try: parser.close() except ParseError: e = sys.exc_info()[1] self.assertNotEqual(None, e.code) self.assertNotEqual(0, e.code) self.assertTrue(isinstance(e.position, tuple)) self.assertTrue(e.position >= (0, 0)) It crashes in expat/xmlparse.c, line 1464: 1459 for (;;) { 1460 BINDING *b = bindings; 1461 if (!b) 1462 break; 1463 bindings = b->nextTagBinding; 1464 FREE(b->uri); // <<<<<<<<<<<<<<< crashes here 1465 FREE(b); 1466 } 1467 } Probably related to the new expat version (issue 31170). |
msg302383 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) *  | Date: 2017-09-17 19:18 |
I can't reproduce a crash. Could you please provide a Python script? |
msg302384 - (view) | Author: Stefan Behnel (scoder) *  | Date: 2017-09-17 20:06 |
Sorry, wrong line number. Was using an installed Py3.7, not a fresh build. However, my crashing installed version is from September 1st, *before* the expat update, which was apparently on September 5th. With a clean debug build, I get a reproducible crash during shutdown, now with the correct line number: Program received signal SIGSEGV, Segmentation fault. PyExpat_XML_ParserFree (parser=0xd01a20) at .../Modules/expat/xmlparse.c:1487 1487 tagList = tagList->parent; (gdb) p taglist No symbol "taglist" in current context. (gdb) list 1482 break; 1483 tagList = freeTagList; 1484 freeTagList = NULL; 1485 } 1486 p = tagList; 1487 tagList = tagList->parent; 1488 FREE(p->buf); 1489 destroyBindings(p->bindings, parser); 1490 FREE(p); 1491 } (gdb) bt #0 PyExpat_XML_ParserFree (parser=0xd01a20) at Modules/expat/xmlparse.c:1487 #1 0x00007fffeec83517 in xmlparser_gc_clear (self=0x7fffeeeb75d8) at Modules/_elementtree.c:3414 #2 xmlparser_dealloc (self=0x7fffeeeb75d8) at Modules/_elementtree.c:3435 #3 0x000000000041fd40 in frame_dealloc (f=0xd017c8) at Objects/frameobject.c:428 #4 0x000000000044f5e5 in tb_dealloc (tb=0x7ffff632d218) at Python/traceback.c:56 #5 0x000000000049c59d in BaseException_clear (self=0x7ffff6567dd8) at Objects/exceptions.c:78 #6 SyntaxError_clear (self=0x7ffff6567dd8) at Objects/exceptions.c:1381 #7 0x0000000000458104 in delete_garbage (old=<optimized out>, collectable=<optimized out>) at Modules/gcmodule.c:759 #8 collect (generation=generation@entry=2, n_collected=n_collected@entry=0x7fffffffd6c8, n_uncollectable=n_uncollectable@entry=0x7fffffffd6d0, nofail=nofail@entry=0) at Modules/gcmodule.c:911 #9 0x0000000000458d2b in collect_with_callback (generation=2) at Modules/gcmodule.c:1020 #10 PyGC_Collect () at Modules/gcmodule.c:1507 #11 0x0000000000439aaf in Py_FinalizeEx () at Python/pylifecycle.c:1002 #12 0x000000000043b3d8 in Py_Exit (sts=sts@entry=0) at Python/pylifecycle.c:1953 #13 0x000000000043f4e8 in handle_system_exit () at Python/pythonrun.c:608 #14 0x000000000043f94d in handle_system_exit () at Python/pythonrun.c:635 #15 PyErr_PrintEx (set_sys_last_vars=set_sys_last_vars@entry=1) at Python/pythonrun.c:618 #16 0x000000000043fc4a in PyErr_Print () at Python/pythonrun.c:514 #17 0x0000000000441475 in PyRun_SimpleFileExFlags (fp=fp@entry=0x9dc050, filename=<optimized out>, closeit=closeit@entry=1, flags=flags@entry=0x7fffffffd8e0) at Python/pythonrun.c:407 #18 0x0000000000441633 in PyRun_AnyFileExFlags (fp=fp@entry=0x9dc050, filename=<optimized out>, closeit=closeit@entry=1, flags=flags@entry=0x7fffffffd8e0) at Python/pythonrun.c:83 #19 0x000000000045666c in run_file (p_cf=0x7fffffffd8e0, filename=0x9973d0 L"test.py", fp=0x9dc050) at Modules/main.c:341 #20 Py_Main (argc=argc@entry=6, argv=argv@entry=0x996020) at Modules/main.c:895 #21 0x000000000041fa2b in main (argc=6, argv=<optimized out>) at ./Programs/python.c:102 I get it in lxml's test suite runs, will try to reduce it to a test script. |
msg302385 - (view) | Author: Stefan Behnel (scoder) *  | Date: 2017-09-17 20:28 |
Minimal reproducer seems to be this: ---------- import xml.etree.ElementTree as etree def test(): parser = etree.XMLParser() try: parser.close() except etree.ParseError as exc: e = exc # must keep local reference! test() ---------- The master gitrev I tested is 132a7d7cdbc7cb89fa1c1f4e8192241c3d68f549 Since this happens during GC and finalisation, it's quite possible that it's not a bug in ET per se. It might just be a prematurely cleared reference in some tp_clear(), or something like that. (Changing ticket title appropriately.) |
msg302411 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2017-09-18 07:00 |
I confirm the crash using attached bug1.py (first example, completed) and bug2.py (second example). |
msg302412 - (view) | Author: Stefan Behnel (scoder) *  | Date: 2017-09-18 07:13 |
Thanks for confirming, Victor. I hadn't realised that the first update of expat was already back in June. That means it's not ruled out yet as a source of this crash. Bisecting is probably a good idea. |
msg302414 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2017-09-18 07:28 |
The bug is at this line: Breakpoint 6, xmlparser_gc_clear (self=0x7ffff7e28c08) at /home/haypo/prog/python/master/Modules/_elementtree.c:3414 static int xmlparser_gc_clear(XMLParserObject *self) { EXPAT(ParserFree)(self->parser); // <--- HERE ... } This function calls XML_ParserFree() twice on the same parser object. The first call is fine and frees the memory. Since we now use Python memory allocators, XML_ParserFree() fills the freed memory with 0xDB byte pattern (when Python is in debug mode). The second XML_ParserFree() call uses freed memory (filled with 0xDB in debug mode). Call 1: a GC collection Breakpoint 6, xmlparser_gc_clear (self=0x7ffff7e28c08) at /home/haypo/prog/python/master/Modules/_elementtree.c:3414 (gdb) up #1 0x0000000000446636 in delete_garbage (collectable=0x7fffffffd9a0, old=0x9b8f90 <_PyRuntime+432>) at Modules/gcmodule.c:759 (gdb) up #2 0x0000000000446ade in collect (generation=2, n_collected=0x7fffffffda30, n_uncollectable=0x7fffffffda28, nofail=0) at Modules/gcmodule.c:911 (gdb) cont Continuing. Call 2: xmlparser_dealloc() Breakpoint 6, xmlparser_gc_clear (self=0x7ffff7e28c08) at /home/haypo/prog/python/master/Modules/_elementtree.c:3414 (gdb) up #1 0x00007ffff0038cb8 in xmlparser_dealloc (self=0x7ffff7e28c08) at /home/haypo/prog/python/master/Modules/_elementtree.c:3435 IMHO it's an obvious bug in Python. The question is more why/how the code didn't crash before? :-) |
msg302415 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2017-09-18 07:32 |
Python 2.7 is not affected because it doesn't implement tp_clear (it doesn't have xmlparser_gc_clear()), only xmlparser_dealloc() calls EXPAT(ParserFree)(self->parser). I'm unable to reproduce the bug in Python 3.5 nor 3.6. bug2.py creates a reference cycle the "except etree.ParseError as exc: e = exc # must keep local reference!" which requires to trigger a garbage collection to clear the "parser" variable. |
msg302416 - (view) | Author: Stefan Behnel (scoder) *  | Date: 2017-09-18 07:34 |
> The question is more why/how the code didn't crash before? :-) Typical case of a Schroedinbug. |
msg302426 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2017-09-18 08:45 |
>> The question is more why/how the code didn't crash before? :-) > > Typical case of a Schroedinbug. I don't believe in the chaos :-) I ran a "git bisect" since January 1st, 2017. Attached bug2.py started to crash since the following commit related to bpo-29049. --- 5a625d0aa6a6d9ec6574ee8344b41d63dcb9897e is the first bad commit commit 5a625d0aa6a6d9ec6574ee8344b41d63dcb9897e Author: INADA Naoki <songofacandy@gmail.com> Date: Sat Dec 24 20:19:08 2016 +0900 Issue #29049: Call _PyObject_GC_TRACK() lazily when calling Python function. Calling function is up to 5% faster. --- This change is also correct. It's more that the change showed a bug which was hidden before. It's just that now the garbage collector breaks the reference cycle differently since frames tracked differently by the GC. |
msg302430 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2017-09-18 09:00 |
Attached PR 3641 fixes bug1.py and bug2.py crashes. Sadly, I failed to write a reliable unit test using bug2.py. The bug requires to trigger the garbage collector in a specific order which depends on how frames are tracked by the GC... |
msg302432 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) *  | Date: 2017-09-18 09:20 |
def test_issue31499(self): def test(): ... test() test.support.gc_collect() |
msg302434 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2017-09-18 09:21 |
Serhiy Storchaka: I tried your pattern, but failed to write a reliable unit test. Can you please write a full patch / test? |
msg302441 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2017-09-18 09:49 |
test_issue31499.diff: Oh great, it works (to reproduce the crash). I modified your test and included it in my PR, I added you as a co-author of my PR ;-) |
msg302447 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2017-09-18 12:29 |
New changeset e727d41ffcd91b21ce82026ec8c8381d34a16209 by Victor Stinner in branch 'master': bpo-31499, xml.etree: Fix xmlparser_gc_clear() crash (#3641) https://github.com/python/cpython/commit/e727d41ffcd91b21ce82026ec8c8381d34a16209 |
msg302449 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2017-09-18 12:48 |
New changeset 8afd7ab12d7f8915b549cf04af384b495ec73d22 by Victor Stinner (Miss Islington (bot)) in branch '3.6': [3.6] bpo-31499, xml.etree: Fix xmlparser_gc_clear() crash (GH-3641) (#3645) https://github.com/python/cpython/commit/8afd7ab12d7f8915b549cf04af384b495ec73d22 |
msg302451 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2017-09-18 13:25 |
The bug is now fixed in Python 3.6 and master. Thanks for the bug report and analysis, Stefan Behnel! |
|
Date | User | Action | Args |
2022-04-11 14:58:52 | admin | set | github: 75680 |
2017-09-18 13:25:23 | vstinner | set | status: open -> closed resolution: fixed messages: + msg302451
stage: patch review -> resolved |
2017-09-18 12:48:26 | vstinner | set | messages: + msg302449 |
2017-09-18 12:29:51 | python-dev | set | pull_requests: + pull_request3639 |
2017-09-18 12:29:40 | vstinner | set | messages: + msg302447 |
2017-09-18 09:49:01 | vstinner | set | messages: + msg302441 |
2017-09-18 09:37:05 | serhiy.storchaka | set | files: + test_issue31499.diff |
2017-09-18 09:21:28 | vstinner | set | messages: + msg302434 |
2017-09-18 09:20:38 | serhiy.storchaka | set | messages: + msg302432 |
2017-09-18 09:00:54 | vstinner | set | messages: + msg302430 versions: + Python 3.6 |
2017-09-18 08:59:06 | vstinner | set | keywords: + patch stage: patch review pull_requests: + pull_request3636 |
2017-09-18 08:45:05 | vstinner | set | messages: + msg302426 |
2017-09-18 07:34:00 | scoder | set | messages: + msg302416 |
2017-09-18 07:32:39 | vstinner | set | messages: + msg302415 |
2017-09-18 07:28:28 | vstinner | set | messages: + msg302414 |
2017-09-18 07:13:52 | scoder | set | messages: + msg302412 |
2017-09-18 07:00:17 | vstinner | set | files: + bug2.py |
2017-09-18 07:00:12 | vstinner | set | files: + bug1.py
messages: + msg302411 |
2017-09-17 20:28:16 | scoder | set | messages: + msg302385 title: ElementTree crash with new expat -> ElementTree crash while cleaning up ParseError |
2017-09-17 20:06:12 | scoder | set | messages: + msg302384 |
2017-09-17 19:18:39 | serhiy.storchaka | set | messages: + msg302383 |
2017-09-17 18:50:07 | jkloth | set | nosy: + jkloth
|
2017-09-17 17:27:08 | scoder | create | |