[ expat-Bugs-596931 ] XML_ParseReset and memory leaks

[noreply@sourceforge.net]

Bugs item #596931, was opened at 2002-08-18 20:25
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110127&aid=596931&group_id=10127

Category: None
Group: None
Status: Open
>Resolution: Fixed
Priority: 5
Submitted By: Nobody/Anonymous (nobody)
Assigned to: Karl Waclawek (kwaclaw)
Summary: XML_ParseReset and memory leaks

Initial Comment:
The problem:
XML_ParseReset function does not reset the parser 
correctly, resulting in memory leaks. These leaks can 
be observed using the windows task manager while 
running the sample code below.  

Expat version: 1.95.4
Platform: win200

Sample Application:

include <string.h>
#include "expat.h"

char buffer[] = "<?xml version=\1.0\?><request 
version=\1.00\></request>";

void main()
{
   XML_Parser parser = XML_ParserCreate(NULL);

   while ( true )
   {
      int ret = XML_Parse( parser, buffer, strlen(buffer ), 1);
      if( ret == 0 )
      {
         abort();
      }

      ret = XML_ParserReset( parser, NULL );
      if( ret == 0 )
      {
         abort();
      }
   }
}


----------------------------------------------------------------------

>Comment By: Karl Waclawek (kwaclaw)
Date: 2002-08-22 11:08

Message:
Logged In: YES 
user_id=290026

Patch applied - seems OK since it passed all my tests
as well as the W3C test suite (with results as expected,
no change from release 1.95.4).

Hopefully that will get more people to test it.
It seems the user community at large will only really
use and test full releases, and to a lesser degree
CVS submissions.

Leave it open for a while, since we may get some
feedback.

----------------------------------------------------------------------

Comment By: Karl Waclawek (kwaclaw)
Date: 2002-08-18 21:53

Message:
Logged In: YES 
user_id=290026

Yes, this is a bug, as discussed on expat-discuss.
I have attached a first patch - still to be tested.

The problem was that dtdDestroy was not called in 
XML_ParserReset, since dtd.scaffold was NULL.
Then, when dtdInit() was later called in parserInit,
the pointers were overwritten - leading to memory leaks.

This was a logic problem - the original reason for checking 
dtd.scaffold was to prevent double-destroying the dtd 
structure, but dtd.scaffold was only set when there was a
content model.

I changed this by introducing the new functions dtdReset() 
and hashTableClear(). There were other logic errors too, that
would not show up in the test case given, but would have
led to memory leaks in other situations:
- freeBindingList and inheritedBindings were overwritten
- tagStack and freeTagList were overwritten
- groupSize and  groupConnector were overwritten

Other changes:
- There is no reason to have dtdDestroy/dtdReset conditional 
  on XML_DTD.
- The unknownEncodingHandler(Data) is not reset anymore,
  since this is not really a dynamic handler anyway.
- I also removed dtdInit() from parserInit() and it is now called 
  separately.
- The return type of XML_ParserReset was changed from int
  to XML_Bool. This should not be much of a problem, since
  it is binary compatible, and XML_ParserReset is new
  and not even documented yet.
- A couple of minor code cleanups were performed too,
  not to confuse the reader.


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110127&aid=596931&group_id=10127