4796 2005-09-01 15:17:19 -0700 leaks of NodeImpl called from HTMLParser::textCreateErrorCheck, seen running webkit tests 2005-09-06 19:35:17 -0700 1 1 1 Unclassified WebKit WebKit Misc. 420+ Mac OS X 10.4 RESOLVED FIXED P2 Normal --- 1 sullivan darin oldest_to_newest 18198 0 sullivan 2005-09-01 15:17:19 -0700 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). 18315 1 kohler 2005-09-02 22:27:52 -0700 (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? 18318 2 kohler 2005-09-02 23:49:01 -0700 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. 18937 3 darin 2005-09-06 19:17:13 -0700 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. 18939 4 darin 2005-09-06 19:35:17 -0700 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.