Re: [Libguestfs] [PATCH v2] v2v: Free XML objects in the correct order.

If you free an xmlDocPtr before any xmlXPathObjectPtrs that reference the doc, you'll get valgrind errors like this: ==7390== Invalid read of size 4 ==7390== at 0x4EB8BC6: xmlXPathFreeNodeSet (xpath.c:4185) ==7390== by 0x4EB8CC5: xmlXPathFreeObject (xpath.c:5492) ==7390== by 0x400A56: main (in /tmp/test) ==7390== Address 0x60c0928 is 8 bytes inside a block of size 120 free'd ==7390== at 0x4C29D2A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==7390== by 0x4E8784F: xmlFreeNodeList (tree.c:3683) ==7390== by 0x4E87605: xmlFreeDoc (tree.c:1242) ==7390== by 0x400A4A: main (in /tmp/test) The following simple test program demonstrates the problem: #include <stdio.h> #include <stdlib.h> #include <assert.h> #include <libxml/xpath.h> int main (int argc, char *argv[]) { xmlDocPtr doc; xmlXPathContextPtr xpathctx; xmlXPathObjectPtr xpathobj; doc = xmlReadMemory ("<test/>", 7, NULL, NULL, XML_PARSE_NONET); assert (doc); xpathctx = xmlXPathNewContext (doc); assert (xpathctx); xpathobj = xmlXPathEvalExpression (BAD_CAST "/test", xpathctx); assert (xpathobj); xmlFreeDoc (doc); xmlXPathFreeObject (xpathobj); xmlXPathFreeContext (xpathctx); exit (EXIT_SUCCESS); } In virt-v2v we were not freeing up objects in the correct order, because we didn't express the dependency between objects at the C level into the OCaml, where the OCaml garbage collector could see those dependencies. For example code like: let doc = ... in let xpathctx = xpath_new_context doc in let xpathobj = xpath_eval_expression xpathctx "/foo" in might end up freeing the 'doc' (xmlDocPtr) if, say, there were no further references to it in the code, even though the 'xpathobj' (xmlXPathObjectPtr) remains live. To avoid this, we change the OCaml-level representation of objects like xpathobj so they contain a reference back to the higher-level objects (xpathctx & doc). Therefore holding an xpathobj means that the doc cannot be freed. However that alone is not quite sufficient. There is a further problem when the program calls Gc.full_major, Gc.compact etc., or even just when xpathctx & doc happen to be freed at the same time. The GC won't necessarily free them in the right order as it knows both need to be freed but doesn't know that one must be freed before the other. To solve this we have to move the finalisers into OCaml code, since the OCaml Gc.finalise function comes with an explicit ordering guarantee (that finalisers are always called in reverse order that they were created), which the C-level finaliser does not. --- [...]