GNOME Bugzilla – Bug 109368
Does not handle out-of-memory
Last modified: 2009-08-15 18:40:50 UTC
In D-BUS there is support for various XML parsers to load the config file. D-BUS has a framework for testing handling of malloc failures; using libxml, the unit test fails with this output: malloc of 2 byte failed xmlNewTextLen : malloc failed malloc of 4 byte failed xmlNewTextLen : malloc failed malloc of 6 byte failed malloc of 2 byte failed xmlNewTextLen : malloc failed xmlNewNsPropEatName : malloc failed Segmentation fault D-BUS basically calls xmlNewTextReaderFilename() then xmlTextReaderRead(). When memory is exhausted, errors should be returned to the application indicating the error, or libxml should have a policy like GLib that it will simply exit on OOM. Printing to stderr and continuing isn't really useful since the application can't recover. For the config file, D-BUS does not really need to handle OOM, but if D-BUS uses XML for other purposes after startup (especially if it uses XML in response to client requests), it would be necessary to return an error code (rather than getting confused or exiting).
Look at the code. I try very hard to not crash on OOM and return an error code. This had been tested in the past and used to work (libxml2 is used in embedded systems !). Seems that some new code breaks this unfortunately. Providing the stack trace where the error occurs would allow to fix this, since you have the framework in place you're in the best position to actually provide it. Daniel
Created attachment 15249 [details] [review] Patch providing test setup for OOM handling
The attached patch adds testOOMlib.[hc] which are some files you can use in test programs to verify OOM handling for various APIs. testOOM.c currently verifies it for the xmlreader API. For the test to be really good, you'd want it to check that you get expected results in all OOM cases. However, right now it is good enough to detect segfaults, memleaks, and stderr printing that occurs on OOM.
Hum, I looked at this, but what do you mean by "get expected results in all OOM cases" If the parser need memory and cannot allocate it it is a fatal processing error. The expected outcome is to stop parsing the document and return an error code. I don't think any other handling is realistic. I could not reproduce the segfault with the current code though I applied your patch as is and tried to reproduce it on a very large document, I got errors reported, I consider this normal, but I'm unclear about what you really expect. Daniel
Okay I think I understand better how it's supposed to work, Daniel
I think what should happen is the same as for any parsing error - an error should be returned to the application.
Okay, I did a serious cleanup based on the testOOM, I just modified the library so that it reuses libxml2 memory aloocation checking layer, which allowed to find easilly the blocks not deallocated. Surprizingly my layer found non-deallocated memory blocks while the OOM test was not reporting any leak :-) This is in CVs and will be in the next release, ultimately I would like to integrate the OOM check to xmllint itself and make it part of the regression tests, but I will finish this later. So I keep the bug as FIXEd but won't close it until the integration is finished ... Thanks, this was a PITA but a really good idea ! Daniel
Most of the error processing problems should be fixed in release libxml2-2.6.0, thanks, Daniel
This should be closed by release of libxml2-2.6.21, thanks, Daniel