Bug 34728 – REGRESSION (before r54472): Various tests in fast/workers are crashing on the buildbot.

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[David Levin]

Specifically, it usually seems to be
  fast/workers/*worker-replace-global-constructor.html 
but there have been others recently.

This is the bug for investigation/patches (cc'ed interested parties, but feel free to remove yourselves as well).

[David Levin]

Forgot to include this.

Current smallest repro steps (on my machine):
   run-webkit-tests LayoutTests/fast/transforms/ LayoutTests/fast/workers/

[Eric Seidel (no email)]

*** Bug 34731 has been marked as a duplicate of this bug. ***

[Eric Seidel (no email)]

As noted in the duplicate, we're hitting an ASSERT in debug builds:
ASSERTION FAILED: m_refCount > 0
(/Volumes/Big/WebKit-BuildSlave/leopard-intel-debug/build/WebKitBuild/Debug/JavaScriptCore.framework/PrivateHeaders/RefCounted.h:68
bool WTF::RefCountedBase::derefBase())

[Andrew Wilson]

It looks like it started happening around 54472 (note the crash in shared-worker-redirect), which doesn't make any sense since it was an unrelated change.

And yet, it was green before that:

http://build.webkit.org/waterfall?last_time=1265580000

[Eric Seidel (no email)]

I looked through the /waterfall history and:
http://build.webkit.org/builders/Leopard%20Intel%20Debug%20(Tests)/builds/10226
from r54472 is the earliest crash I could find.

[David Levin]

Created attachment 48379 [details]
the crash dump.

Fix coming soon-ish.

[David Levin]

Created attachment 48380 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 48380 [details]
Patch

When did this actually start crashing?  do we know what change caused this?

[David Levin]

Created attachment 48381 [details]
Patch

[David Levin]

(In reply to comment #8)
> (From update of attachment 48380 [details])
> When did this actually start crashing?  do we know what change caused this?

My fix was based on looking at the crash and realizing that the object being deref'ed was just pretending to be ref counted, so I fixed that after talking to Gavin about it.

I didn't narrow don't the exact change that exposed this underlying issue (but I was guessing that it was http://trac.webkit.org/changeset/54400 -- I didn't confirm that in any way but I could attempt another build without just this one change to see if my guess is right).

[Gavin Barraclough]

Comment on attachment 48381 [details]
Patch

Looks awesome, thank you!

[David Levin]

Committed as http://trac.webkit.org/changeset/54525

[David Levin]

(In reply to comment #10)
> (In reply to comment #8)
> > (From update of attachment 48380 [details] [details])
> > When did this actually start crashing?  do we know what change caused this?
>
> I was guessing that it was http://trac.webkit.org/changeset/54400 

I started with a git branch that had the crash repro'ing. Then I rolled out r54400 locally. Did the build.
Ran the tests and they passed.

So the revision that started the issue for workers is confirmed if that is of help to folks.

[Dmitry Titov]

I wonder if we want to have an ASSERT like this:

    ~RefCountedBase()
    {
       ASSERT(m_deletionHasBegun);
    }

That would prevent deleting of the RefCounted objects other then via deref().  We do have cases of that, for example StyleBase is RefCounted but StyleList that derives from is deleted by 'delete'.