RESOLVED FIXED 4796
leaks of NodeImpl called from HTMLParser::textCreateErrorCheck, seen running webkit tests
https://bugs.webkit.org/show_bug.cgi?id=4796
Summary leaks of NodeImpl called from HTMLParser::textCreateErrorCheck, seen running ...
John Sullivan
Reported 2005-09-01 15:17:19 PDT
This bug is also in Radar as <rdar://4232812> This is split off from 4665. This is one of the many leaks found with these steps: 1. Build a development build of tip-of-tree WebKit 2. use run-webkit-tests --leaks Leak: 0x1d5ac780 size=48 0x013f85b0 0x00000000 0x00000000 0x1d5d1990 .?...........].. 0x00000000 0x00000000 0x00000000 0x00000000 ................ 0x00000000 0x00000000 0x1f30fc90 0x00000000 .........0...... Call stack: [thread 239f]: | 0x0 | start | _start | main | dumpRenderTree | -[NSRunLoop runMode:beforeDate:] | CFRunLoopRunSpecific | __CFRunLoopRun | __CFRunLoopDoSources0 | _sendCallbacks | -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks] | -[NSURLConnection (NSURLConnectionInternal) _sendDidReceiveDataCallback] | -[WebLoader connection:didReceiveData:lengthReceived:] | -[WebMainResourceLoader didReceiveData:lengthReceived:] | -[WebLoader didReceiveData:lengthReceived:] | - [WebMainResourceLoader addData:] | -[WebDataSource(WebPrivate) _receivedData:] | - [WebHTMLRepresentation receivedData:withDataSource:] | -[WebBridge receivedData:textEncodingName:] | -[WebCoreBridge addData:] | KWQKHTMLPart::addData(char const*, int) | KHTMLPart::write(char const*, int) | khtml::HTMLTokenizer::write(khtml::TokenizerString const&, bool) | khtml::HTMLTokenizer::processToken() | HTMLParser::parseToken(khtml::Token*) | HTMLParser::getNode(khtml::Token*) | HTMLParser::textCreateErrorCheck(khtml::Token*, DOM::NodeImpl*&) | DOM::NodeImpl::operator new(unsigned long) | khtml::main_thread_malloc (unsigned long) | malloc This leak occurred 4 times in the attachment in 4665 I still see this one today (most of the other leaks from 4665 have now been fixed).
Attachments
Kurt Kohler
Comment 1 2005-09-02 22:27:52 PDT
(In reply to comment #0) > This bug is also in Radar as <rdar://4232812> > > This is split off from 4665. This is one of the many leaks found with these steps: > > 1. Build a development build of tip-of-tree WebKit > 2. use run-webkit-tests --leaks > > Leak: 0x1d5ac780 size=48 > 0x013f85b0 0x00000000 0x00000000 0x1d5d1990 .?...........].. > 0x00000000 0x00000000 0x00000000 0x00000000 ................ > 0x00000000 0x00000000 0x1f30fc90 0x00000000 .........0...... > Call stack: [thread 239f]: | 0x0 | start | _start | main | dumpRenderTree | -[NSRunLoop > runMode:beforeDate:] | CFRunLoopRunSpecific | __CFRunLoopRun | __CFRunLoopDoSources0 | > _sendCallbacks | -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks] | -[NSURLConnection > (NSURLConnectionInternal) _sendDidReceiveDataCallback] | -[WebLoader > connection:didReceiveData:lengthReceived:] | -[WebMainResourceLoader > didReceiveData:lengthReceived:] | -[WebLoader didReceiveData:lengthReceived:] | - > [WebMainResourceLoader addData:] | -[WebDataSource(WebPrivate) _receivedData:] | - > [WebHTMLRepresentation receivedData:withDataSource:] | -[WebBridge > receivedData:textEncodingName:] | -[WebCoreBridge addData:] | KWQKHTMLPart::addData(char const*, > int) | KHTMLPart::write(char const*, int) | khtml::HTMLTokenizer::write(khtml::TokenizerString const&, > bool) | khtml::HTMLTokenizer::processToken() | HTMLParser::parseToken(khtml::Token*) | > HTMLParser::getNode(khtml::Token*) | HTMLParser::textCreateErrorCheck(khtml::Token*, > DOM::NodeImpl*&) | DOM::NodeImpl::operator new(unsigned long) | khtml::main_thread_malloc > (unsigned long) | malloc > > This leak occurred 4 times in the attachment in 4665 > > I still see this one today (most of the other leaks from 4665 have now been fixed). I've been looking at that same one and I may see the problem. In HTMLParser::getNode() "result" is potentially set twice. When it's set the second time, I don't see any guarantee that you won't be overwriting a non-null pointer. I think there should be a "delete result" just before the second time "result" is set. I can submit a patch, but it seems pretty trivial (assuming I'm right!) By the way, wouldn't it be more straightforward to have getNode return a SmartPtr rather than converting the raw pointer after the (single) call to getNode?
Kurt Kohler
Comment 2 2005-09-02 23:49:01 PDT
Unfortunately the change I suggested doesn't fix the leak. Sorry. I'd still suggest an "assert(!result)" just before the assignment to result however.
Darin Adler
Comment 3 2005-09-06 19:17:13 PDT
This function is where all text nodes are created while parsing. I'm pretty sure that this is the same leak I fixed in my fix for bug 4797.
Darin Adler
Comment 4 2005-09-06 19:35:17 PDT
Checked in a fix that should fix the leak described in this bug, as well as two others. The three bugs are bug 4795, bug 4796, and bug 4797.
Note You need to log in before you can comment on or make changes to this bug.