1374675 - Crash in mozilla::dom::IDBFactory::CreateForWindow

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is 
report bp-255e059d-91c1-467a-b602-2881d0170620.
=============================================================

Seen while looking at crash data - crashes started using 20170620030208: http://bit.ly/2sSfwEu

Possible regression range based on Build ID: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=95543bdc59bd038a3d5d084b85a4fec493c349ee&tochange=7a6baa6cca3292e8099e652b64d27e74df560874

Bug 1352401 touched some code in this area. ni on :bevis

Comment 1

[Jan Varga [:janv]]

It seems globalWindow->GetScriptableTop() can return nullptr.

Comment 2

[Bevis Tseng[:bevis][:btseng](Exited)]

In this case, we should re-consider to count individually in IDBFactory::UpdateActiveXxxCount() using TabGroup & mWindow instead of mTopWindow.
I'll backout bug 1352401 first and revise the UpdateActive{Database|Transaction)Count() method again.

Comment 3

[Jan Varga [:janv]]

IIRC, TabGroup was accessed through mTopWindow in your previous version of the patch, that's why I suggested to update both counters in the window code.
Anyway, mWindow has access to the TabGroup too.

Comment 4

[Bevis Tseng[:bevis][:btseng](Exited)]

Yes, my concern is that TabGroup is always accessible from mTopWindow so the counter could be updated correctly.
However, for the counter in the TopWindow that is used by TimeoutManager, how could we update the counters correctly if GetScriptableTop() is null? :(

Comment 5

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Bevis Tseng [:bevis][:btseng] from comment #4)
> Yes, my concern is that TabGroup is always accessible from mTopWindow so the
                                                             ^ Sorry, it's mWindow instead.
> counter could be updated correctly.
> However, for the counter in the TopWindow that is used by TimeoutManager,
> how could we update the counters correctly if GetScriptableTop() is null? :(

Comment 6

[Bevis Tseng[:bevis][:btseng](Exited)]

But you are right, we should keep the counting be implemented inside nsGlobalWindow but count with mWindow instead of mTopWindow in IDBFactory.

Comment 7

[Jan Varga [:janv]]

So, can you just call mWindow->UpdateXXX() and let that code decide what to update ?
The method would always update its TabGroup counter and only update its top window counter if the top window exists.
It would be even nicer if we can get rid of mTopWindow in IDBFactory.

Comment 8

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Jan Varga [:janv] from comment #7)
> So, can you just call mWindow->UpdateXXX() and let that code decide what to
> update ?
> The method would always update its TabGroup counter and only update its top
> window counter if the top window exists.
> It would be even nicer if we can get rid of mTopWindow in IDBFactory.

Yes, that's what I plan to do but maybe update the counter directly to the TimeoutManager of the mWindow. (Because it's still scary to me that we have a valid inner window and its TimeoutManager but we don't provide valid counter information to its TimeoutManager when ScriptableTop is not available which could block the IDB access in other window if timer throttling is enabled.

Comment 9

[Jan Varga [:janv]]

Ok, sounds good.

Comment 10

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/23cd94436e78
Backed out changeset 9c62253ad1a3 
https://hg.mozilla.org/integration/mozilla-inbound/rev/2566cc1e0dd7
Backed out changeset 70d068cc946d

Comment 11

[Caspy7]

It looks like you folks may have a handle on this, but in case it's helpful, there are two users here (poster and a commenter) reporting similar problems as well as a user in the #nightly IRC channel reporting the same.
https://www.reddit.com/r/firefox/comments/6ifrza/nightly_has_stopped_working_every_time_i_close/
They include various bits of information about their systems and the crashes.

The reporter from #nightly induced a crash and provided this crash report: https://crash-stats.mozilla.com/report/index/5255f998-26f5-4b93-990f-6efe80170621

Comment 12

[Peter Gervai]

(There are plenty of crash reports, referring to this bug.)

Comment 13

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Peter Gervai from comment #12)
> (There are plenty of crash reports, referring to this bug.)

Sorry about that.
It has been backed out in mozilla-inbound and shall be backed out from mozilla-central soon.

Comment 14

[Peter Gervai]

Seen the backout. No grudge here. Just had to leave nightly until this gets fixed and pushed out since it became absolutely unusable. It's a weird feeling to be on FF-ESR [and having all my addons magically work again]. :-]

Comment 16

[Bob Clary [:bc] (inactive)]

This was also seen on two urls in Bughunter:
https://elise.com.ua/2015/01/11/za-samoj-krovavoj-terroristicheskoj-o/
https://www.theatlantic.com/magazine/archive/2017/06/when-your-child-is-a-psychopath/524502/?utm_source=twb

bug 1375066 has a testcase attachment 8880001 [details]

Comment 17

[Bob Clary [:bc] (inactive)]

Attachment: assertion stack

Forgot to mention:

Assertion failure: topOutterWindow, at /home/worker/workspace/build/src/dom/indexedDB/IDBFactory.cpp:168

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Merged the backout:
https://hg.mozilla.org/mozilla-central/rev/23cd94436e78
https://hg.mozilla.org/mozilla-central/rev/2566cc1e0dd7

Comment 19

[Bevis Tseng[:bevis][:btseng](Exited)]

Will land the patch again by fixing bug 1352401.