Issue with _IncrementalFileWriter._handle_error
Hi, (I'm working on getting lxml incremental writer to write on a twisted http request object) This is from lxml trunk, _IncrementalFileWriter in serializer.pxi cdef _handle_error(self, int error_result): if error_result != xmlerror.XML_ERR_OK: if self._writer is not None: self._writer._exc_context._raise_if_stored() _raiseSerialisationError(error_result) I could be wrong but I don't think _IncrementalFileWriter should have a separate _writer attribute. Here's the relevant bit of traceback i'm getting: File "serializer.pxi", line 637, in lxml.etree.xmlfile.__exit__ (src/lxml/lxml.etree.c:110916) File "serializer.pxi", line 870, in lxml.etree._IncrementalFileWriter._close (src/lxml/lxml.etree.c:114606) File "serializer.pxi", line 874, in lxml.etree._IncrementalFileWriter._handle_error (src/lxml/lxml.etree.c:114663) Is this a typo/bug or am I missing something? Best, Burak
Burak Arslan, 23.12.2013 11:29:
(I'm working on getting lxml incremental writer to write on a twisted http request object)
This is from lxml trunk, _IncrementalFileWriter in serializer.pxi
cdef _handle_error(self, int error_result): if error_result != xmlerror.XML_ERR_OK: if self._writer is not None: self._writer._exc_context._raise_if_stored() _raiseSerialisationError(error_result)
I could be wrong but I don't think _IncrementalFileWriter should have a separate _writer attribute.
Here's the relevant bit of traceback i'm getting:
File "serializer.pxi", line 637, in lxml.etree.xmlfile.__exit__ (src/lxml/lxml.etree.c:110916) File "serializer.pxi", line 870, in lxml.etree._IncrementalFileWriter._close (src/lxml/lxml.etree.c:114606) File "serializer.pxi", line 874, in lxml.etree._IncrementalFileWriter._handle_error (src/lxml/lxml.etree.c:114663)
Is this a typo/bug or am I missing something?
A bug. Thanks for finding it. https://github.com/lxml/lxml/commit/9693d735e014a39fe697c5c76177287ffdc2a06d Stefan
Hi, I got another one for you related to this bug. This probably is because I'm trying to write to an _IncrementalWriter outside of the with block. I'll eventually fix that. https://gist.github.com/plq/8113509 Do note that I have the following towards the end: (gdb) p __pyx_v_self $1 = (struct __pyx_obj_4lxml_5etree__IncrementalFileWriter *) 0x1671e08 (gdb) p __pyx_v_self->_c_out $2 = (xmlOutputBuffer *) 0x0 (gdb) ... and also the python code for line 113538 in src/lxml/lxml.etree.c When I revert 9693d735e014a39fe697c5c76177287ffdc2a06d, I get: python2.7: Objects/typeobject.c:3976: PyType_Ready: Assertion `type->tp_dict != ((void *)0)' failed. Ping me if you need anything else. Best, Burak
Hi, Here's a testcase that segfaults CPython: https://gist.github.com/plq/8164989 This is how Spyne's output pipeline works at its heart. Relevant: https://mail.python.org/pipermail/python-list/2013-December/663477.html Also see: http://twistedmatrix.com/pipermail/twisted-python/2013-December/027869.html Best, Burak On 12/24/13 15:47, Burak Arslan wrote:
Hi,
I got another one for you related to this bug. This probably is because I'm trying to write to an _IncrementalWriter outside of the with block. I'll eventually fix that.
https://gist.github.com/plq/8113509
Do note that I have the following towards the end:
(gdb) p __pyx_v_self $1 = (struct __pyx_obj_4lxml_5etree__IncrementalFileWriter *) 0x1671e08 (gdb) p __pyx_v_self->_c_out $2 = (xmlOutputBuffer *) 0x0 (gdb)
... and also the python code for line 113538 in src/lxml/lxml.etree.c
When I revert 9693d735e014a39fe697c5c76177287ffdc2a06d, I get:
python2.7: Objects/typeobject.c:3976: PyType_Ready: Assertion `type->tp_dict != ((void *)0)' failed.
Ping me if you need anything else.
Best, Burak _________________________________________________________________ Mailing list for the lxml Python XML toolkit - http://lxml.de/ lxml@lxml.de https://mailman-mail5.webfaction.com/listinfo/lxml
Burak Arslan, 28.12.2013 23:28:
Here's a testcase that segfaults CPython: https://gist.github.com/plq/8164989
Thanks! https://github.com/lxml/lxml/commit/2188421b7e64265f85f27012727a4881df3e1e68 Here's the test case I distilled from it: https://github.com/lxml/lxml/commit/6a80420aafcdcf7e45edc2e743e016d0b7f4b154 Stefan
On 01/01/14 21:28, Stefan Behnel wrote:
Burak Arslan, 28.12.2013 23:28:
Here's a testcase that segfaults CPython: https://gist.github.com/plq/8164989 Thanks!
https://github.com/lxml/lxml/commit/2188421b7e64265f85f27012727a4881df3e1e68
Here's the test case I distilled from it:
https://github.com/lxml/lxml/commit/6a80420aafcdcf7e45edc2e743e016d0b7f4b154
Hi Stefan, Thanks for the fix. Will these fixes go to 3.2.x as well? I'll have to tweak Spyne docs accordingly. Happy new year :) Best, Burak
Burak Arslan, 02.01.2014 09:38:
On 01/01/14 21:28, Stefan Behnel wrote:
Burak Arslan, 28.12.2013 23:28:
Here's a testcase that segfaults CPython: https://gist.github.com/plq/8164989 Thanks!
https://github.com/lxml/lxml/commit/2188421b7e64265f85f27012727a4881df3e1e68
Here's the test case I distilled from it:
https://github.com/lxml/lxml/commit/6a80420aafcdcf7e45edc2e743e016d0b7f4b154
Thanks for the fix. Will these fixes go to 3.2.x as well? I'll have to tweak Spyne docs accordingly.
It's in the lxml-3.2 branch already, there'll be a bug fix release at some point. Still, you should try to make Spyne more robust in error cases. What you get now is an exception instead of a crash, but with incomplete output. That's certainly better than before, but it may still not be what you want. I've considered to at least close all currently open tags on errors, but I'm not sure that it would be a good idea. In the case of an I/O error, this will just lead to more errors, and in the case that there's a remote site reading synchronously, it may end up misinterpreting its input stream as complete, at least at the XML level, although it quite likely isn't. Better fail loudly. Or make it an option to xmlfile() to do it or not. Stefan
On 01/02/14 11:27, Stefan Behnel wrote:
On 01/01/14 21:28, Stefan Behnel wrote:
Burak Arslan, 28.12.2013 23:28:
Here's a testcase that segfaults CPython: https://gist.github.com/plq/8164989 Thanks!
https://github.com/lxml/lxml/commit/2188421b7e64265f85f27012727a4881df3e1e68
Here's the test case I distilled from it:
https://github.com/lxml/lxml/commit/6a80420aafcdcf7e45edc2e743e016d0b7f4b154 Thanks for the fix. Will these fixes go to 3.2.x as well? I'll have to tweak Spyne docs accordingly. It's in the lxml-3.2 branch already, there'll be a bug fix release at some
Burak Arslan, 02.01.2014 09:38: point.
Still, you should try to make Spyne more robust in error cases. What you get now is an exception instead of a crash, but with incomplete output. That's certainly better than before, but it may still not be what you want.
That's fine. My tests are all green, which means in theory Spyne users don't have anything to worry about. But I'm just a puny mortal I just want to make users aware that there is a risk.
I've considered to at least close all currently open tags on errors, but I'm not sure that it would be a good idea. In the case of an I/O error, this will just lead to more errors, and in the case that there's a remote site reading synchronously, it may end up misinterpreting its input stream as complete, at least at the XML level, although it quite likely isn't. Better fail loudly. Or make it an option to xmlfile() to do it or not.
I woudn't bother. I'd prefer for it to fail without attempting any further action as well. Best, Burak
participants (2)
-
Burak Arslan -
Stefan Behnel