Re: [xslt] Segmentation fault in the latest release by processing some DocBook XML files

[Daniel Veillard <veillard redhat com>]

On Thu, Jul 26, 2007 at 12:36:27PM -0700, William M. Brack wrote:
> Daniel Veillard wrote:
> >   Bill was looking at it yesterday.
> > Any chance you could reduce this to a simpler example than the full
> > DocBook stylesheets ? That would really make it less scary and help
> > fixing it.
> >   I can see how removing the namespace while it could still be in use
> > in the subtree could really mess things up. It don't think one can
> > xmlFreeNs(ns) at that point without taking huge risks.
> >
> >   Bill, do you remember wy you wanted to get rid of it ?
> >
> 
> I remember it fairly well (which, in itself, is unusual for a person with
> a memory as mature mine).  While checking on some other unrelated
> problem, I noticed that, when running in the special test environment
> which you and I use for all compilations, there was an error being
> generated by debugXML concerning namespaces related to
> exclude-result-prefixes processing.  I discussed it with you briefly on
> the Chat channel, and you suggested that it might be better to leave it
> alone, since 1) this only occurs when debugXML has been enabled, 2) there
> had never been any problem reported with it, and 3) my suggested "fix"
> for this included deleting the troublesome (duplicate) node, which you
> warned me might cause trouble with pointers to the node which may have
> been previously created.
> 
> However, being a "purist" at heart, I decided to go ahead and put in the
> "fix", which very nicely took care of the debugXML warning.  Further, I
> ran through all of the regression tests (including several for DocBook)
> and convinced myself that there would be no problem.  Obviously, this has
> been proven to be badly mistaken :-(.
> 
> I am, therefore, reverting the xslt.c code to it's previous state, and
> will just close my eyes when the debugXML warning shows up again in the
> future!

  Okay, I think I now have all the pieces from the puzzle, some I had
forgotten, so let's try to recap the whole story.
  libxslt when doing the stylesheet compilation does that thing of moving
some namespace definition on the document node to avoid propagating them
if they are on the exclude-result-prefixes list. It's an hack which works
but is simpler than checking at execution each time one may want to 
copy a namespace declaration. now this is bad from an XML tree point of
view, and when running in debug mode the free function of libxml2 calls
that checking routine which verifies that everything is all set, that
test itself found numerous bugs in the past. And of course when you
free the stylesheet, the associated doc(s) go though that and the checking
complains  ... rightly.
  Freeing an xmlNsPtr must alway be done very carefully because it could be
used by the subtree under the element holding it or by one of the attributes
on the element. So freeing whithout doing that full check, was dangerous, 
and of course someone finally hit the problem. So either we ignore the
error when running in debug mode, or we try to fix things back before
freeing the tree. One way that could be done would be: 
    - to save the element holding the namespace in the xmlNs _private
      before moving it
    - just before freeing the document in the stylesheet freeing routines
      move them back based on the value of _private
the risk of that approach is if the application uses the _private of the
stylesheet doc namespace, that could break some fancy stuff. A safer way
might be to alloate an array of (xmlNsPtr, xmlNodePtr) in stylesheets to
remember the original locations and again move them back at freeing time.
The second approach is probably not much more complex and certainly safer.
Now is that worth it ? We can certainly live with some warnings in some
test cases when running in debug mode :-)

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/