WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Add attachment
proposed patch, testcase, etc.
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.
Top of Page
Format For Printing
XML
Clone This Bug