970767 - Null deref crashes in workers in NS_CycleCollectorSuspect3

Status: RESOLVED INCOMPLETE

(Core :: DOM: Workers, defect)

Description

[Andrew McCreight [:mccr8]]

It looks like this is a top-10 crash on beta.  I looked at about 6 of these, and in most of these cases, it looks like sCollectorData.get() is returning null on a worker thread.

Comment 1

[Andrew McCreight [:mccr8]]

Maybe we could change the check here to
  if (MOZ_LIKELY(data && data->mCollector)) {

The stacks I looked at didn't look distinctly shutdown-y though.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Andrew can you take this please?  I'm swamped.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Why do we try to use null sCollectorData.get(). That is worrisome. If we add null checks, will we
end up leaking?

Comment 4

[Andrew McCreight [:mccr8]]

Good point, yes, we most likely would, which is bad.  I guess the question is, why are we shutting down the worker CC when we still have stuff to run in the event loop?  Maybe shutting things down in some way injects stuff into the event loop that causes more stuff to get destroyed?  (Sorry if my terminology is wrong.)

Comment 5

[investfund2007]

Face the same issue on xulrunner 27.0.1 in the first 2-3 minutes of work.
Adding check on null data would probably won't cause leaks because in that case it would go to SuspectAfterShutdown which would cleanup CycleCollectable.
I can rebuild xulrunner with this fix but how can I detect if there are any leaks?
Will running tests help?

Comment 6

[investfund2007]

And would I face the same crashes in xulrunner 26?

Comment 7

[investfund2007]

After adding check on null data xulrunner still crashes. Now at 
nsThread::ProcessNextEvent(bool mayWait, bool *result)
{
...
NOTIFY_EVENT_OBSERVERS(AfterProcessNextEvent, (this, mRunningEvent)); //Crashes here
...
}
By the way Firefox 27.0.1 (last stable release) also crashes sometimes, but I can't debug it because of the crashreporter

Comment 8

[investfund2007]

Please ignore comment 7. 
Crash of NOTIFY_EVENT_OBSERVERS was caused by creating a thread in my xpcom component and crash in Firefox 27.0.1 according to about:crashes is caused by Bug 952721
Adding check on null data do fix the error. So the main question is how to check if I have memory leak?

Comment 9

[investfund2007]

I've ended up with upgrade to 28 branch which is working without any modifications.

Comment 10

[Andrew McCreight [:mccr8]]

I haven't seen these crashes recently.