Closed Bug 1336242 Opened 8 years ago Closed 8 years ago

LocalStorage bfcache FireDelayedDOMEvents causes null-pointer reads [Crash in NS_strcmp]

Categories

(Core :: DOM: Core & HTML, defect)

Unspecified
Windows 10
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox52 --- unaffected
firefox53 --- unaffected
firefox54 --- fixed

People

(Reporter: marcia, Assigned: asuth)

References

Details

(Keywords: crash, regression)

Crash Data

This bug was filed from the Socorro interface and is report bp-f75e0395-b72e-4d29-9d08-ce5852170202. ============================================================= Seen while looking at trunk crash stats - crashes started using 20170202030211 build: http://bit.ly/2jCBO9J Possible regression range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1d025ac534a6333a8170a59a95a8a3673d4028ee&tochange=f985243bb630b2c78cd57731c8d8ab191aa09527
These stacks look pretty bogus. On 54, I see 152 crashes that supposedly are coming from nsImageRenderer::ComputeObjectAnchorPoint, though I don't see how that could happen. 80 via MD_CURRENT_THREAD, which also doesn't make sense. Then another 60 or so via nsGlobalWindow::FireDelayedDOMEvents, which maybe is possible. (I actually hit this variant myself. I had just closed a tab or switched tabs, then hit back a bunch of times.)
Here's an example of a crash inside FireDelayeDOMEvents: bp-489caca2-e9b2-4a56-887f-485d92170202 Andreas, could this be related to bug 1313864? It is in the regression range and at least sounds similar.
Flags: needinfo?(afarre)
See Also: → 1336229
(In reply to Andrew McCreight [:mccr8] from comment #2) > Here's an example of a crash inside FireDelayeDOMEvents: > bp-489caca2-e9b2-4a56-887f-485d92170202 > > Andreas, could this be related to bug 1313864? It is in the regression range > and at least sounds similar. I'm not sure, but I think this crash is derived from Bug 1285898. In nsGlobalWindow::FireDelayedDOMEvents(), nsGlobalWindow::Observe is called with *aData = nullptr and hit crash in Observe (line 11823) if (!NS_strcmp(aData, u"sessionStorage"))
(In reply to Toshihiro Yamada from comment #3) > I'm not sure, but I think this crash is derived from Bug 1285898. > In nsGlobalWindow::FireDelayedDOMEvents(), nsGlobalWindow::Observe is called > with *aData = nullptr and hit crash in Observe (line 11823) if > (!NS_strcmp(aData, u"sessionStorage")) Ah, good catch! Yes, that makes sense.
Blocks: 1285898
Flags: needinfo?(afarre) → needinfo?(asuth)
Keywords: regression
See Also: → 1336323
Component: XPCOM → DOM
Indeed, excellent detective work! The ironic thing is that those pending storage events don't even work right. Never have[1]. And I was just about to file the spin-off bug for that since I noticed them in bug 1285898. Also, they apparently leak. I don't see where we clear mPendingStorageEvents. I'm half tempted to just nuke the broken feature... 1: At least, not for content that wants the standard "storage" events. I think the crash makes it clear we have no automated tests covering the bfcache case, but there could be some since the localStorage tests I found seem to use a hacky simplification by depending on the "MozLocalStorageChanged" event sent for events sourced from the self-same document.
Assignee: nobody → bugmail
Status: NEW → ASSIGNED
Summary: Crash in NS_strcmp → LocalStorage bfcache FireDelayedDOMEvents causes null-pointer reads [Crash in NS_strcmp]
I had philor backout bug 1285898 and I'll fix-up the bfcache case and add a test before we re-land. Backout noted at https://bugzilla.mozilla.org/show_bug.cgi?id=1285898#c36 and is commit https://hg.mozilla.org/mozilla-central/rev/2aede0a97bc685e163196cc451b947a04ae6a598.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Flags: needinfo?(asuth)
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.