You need to
before you can comment on or make changes to this bug.
On Leopard I am seeing this test reproducibly crashing. In a debug build, it occasionally generates an assertion failure:
ASSERTION FAILED: _beginCount >= 0
With guard malloc enabled, it crashes 100% of the time with the following trace:
Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0xd7c47fe4
0x00008384 in -[EventSendingController mouseUp] (self=0xd7c47fe0, _cmd=0x19094) at /Volumes/Data/Home/Documents/Work/WebKit-git/OpenSource/WebKitTools/DumpRenderTree/mac/EventSendingController.mm:249
249 down = NO;
#0 0x00008384 in -[EventSendingController mouseUp] (self=0xd7c47fe0, _cmd=0x19094) at /Volumes/Data/Home/Documents/Work/WebKit-git/OpenSource/WebKitTools/DumpRenderTree/mac/EventSendingController.mm:249
#1 0x937bfb7d in __invoking___ ()
#2 0x937bf568 in -[NSInvocation invoke] ()
#3 0x00008c78 in +[EventSendingController replaySavedEvents] (self=0x2a8c0, _cmd=0x1c404) at /Volumes/Data/Home/Documents/Work/WebKit-git/OpenSource/WebKitTools/DumpRenderTree/mac/EventSendingController.mm:342
#4 0x00008159 in -[EventSendingController mouseUp] (self=0xd7c47fe0, _cmd=0x19094) at /Volumes/Data/Home/Documents/Work/WebKit-git/OpenSource/WebKitTools/DumpRenderTree/mac/EventSendingController.mm:226
#5 0x937bfb7d in __invoking___ ()
#6 0x937bf568 in -[NSInvocation invoke] ()
Full crash trace will be attached in a more readable format.
Created an attachment (id=18325) [details]
This is currently crashing for me, and on the buildbot.
Caused by <http://trac.webkit.org/projects/webkit/changeset/29266>.
Haven't been able to reproduce the crash on Linux. I'll try on Mac shortly.
The test has a bug:
It looks like someone is pointing to the old Document after it gets deallocated.
I don't know if this bug can be triggered outside of a LayoutTest. I don't know of a programmatic way to click an anchor element in WebKit (Opera provides a click() method). document.location and window.open load asynchronously. The remaining possibility is the submit() method on forms, which does run synchronously, but I couldn't get it to crash (need to find code somewhere that holds a stale pointer to a document).
Created an attachment (id=18335) [details]
Fixes crashing test
I've disabled this test for now in r29326
(From update of attachment 18335 [details])
I'm confused. This doesn't seem right. Either replacing the document synchronously is OK or it's not. If it's OK, we should figure out the real reason the crash is happening. If it's not, then there's no reason to have a FIXME comment.
We can continue investigating to see what exactly is causing the crash. There are a couple of factors to consider:
1) You'll probably want to ship r29266 as a security update. The attached patch attempts to revert r29266 to the minimal change necessary to fix the security bug. Hopefully this will minimize the chance of shipping a bad security update.
(In reply to comment #6)
> I don't know if this bug can be triggered outside of a LayoutTest. I don't
> know of a programmatic way to click an anchor element in WebKit (Opera provides
> a click() method).
You can do it in WebKit by dispatching a mouse click DOM event to the element. When the document is replaced, scriptable objects are invalidated, which destroys the underlying runtime objects (in the case of the bug, the event sender object). So I do not know if the bug can be triggered without a native object, but I think it can happen with plug-ins as well, if they trigger the replacement.
Created an attachment (id=18361) [details]
Protect native objects from deletion by self-invalidation
(From update of attachment 18335 [details])
Thanks for taking a look at this. Yours is a better fix than mine.
(From update of attachment 18361 [details])
The test is crashing again, with a different backtrace. <http://build.webkit.org/results/trunk-mac-ppc-release/10257/results.html>. Investigating.
I was wronf(In reply to comment #9)
> Either replacing the document
> synchronously is OK or it's not.
According to Maciej and Geoff on irc, it is not OK.
Given the above, the fix should be along the lines of Adam Barth's patch.
(In reply to comment #17)
> I was wronf(In reply to comment #9)
> > Either replacing the document
> > synchronously is OK or it's not.
> According to Maciej and Geoff on irc, it is not OK.
The comment needs to be corrected then. It says "FIXME: We should do this synchronously." But we should not!
Fixed in <http://trac.webkit.org/projects/webkit/changeset/29432> (but tests results not rolled back to reflect the old behavior).
(In reply to comment #21)
Thanks. That's bug 16855.