Closed Bug 1019627 Opened 5 years ago Closed 5 years ago

Intermittent test_usedSpace.html | application crashed [@ nsIFileToJsval(nsPIDOMWindow*, DeviceStorageFile*)] (after: "Assertion failure: aWindow, at nsDeviceStorage.cpp:1667")

Categories

(Core :: DOM: Device Interfaces, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla33
Tracking Status
firefox31 --- fixed
firefox32 --- fixed
firefox33 --- fixed
firefox-esr24 --- unaffected
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: cbook, Assigned: xyuan)

References

()

Details

(Keywords: crash, intermittent-failure)

Attachments

(2 files, 2 obsolete files)

Rev5 MacOSX Mountain Lion 10.8 mozilla-central debug test mochitest-2 on 2014-06-03 00:28:56 PDT for push f28005b84ed0

slave: talos-mtnlion-r5-008

https://tbpl.mozilla.org/php/getParsedLog.php?id=40933526&tree=Mozilla-Central



Assertion failure: aWindow, at /builds/slave/m-cen-osx64-d-0000000000000000/build/dom/devicestorage/nsDeviceStorage.cpp:1667
TEST-UNEXPECTED-FAIL | /tests/dom/devicestorage/test/test_usedSpace.html | application terminated with exit code 1
PROCESS-CRASH | /tests/dom/devicestorage/test/test_usedSpace.html | application crashed [@ nsIFileToJsval(nsPIDOMWindow*, DeviceStorageFile*)]

0:35:53     INFO -  Crash reason:  EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
00:35:53     INFO -  Crash address: 0x0
00:35:53     INFO -  Thread 0 (crashed)
00:35:53     INFO -   0  XUL!nsIFileToJsval(nsPIDOMWindow*, DeviceStorageFile*) [nsDeviceStorage.cpp:f28005b84ed0 : 1666 + 0x0]
00:35:53     INFO -      rbx = 0x00007fff7ce48c68   r12 = 0x000000010059e8b0
00:35:53     INFO -      r13 = 0x00007fff5fbfd3c7   r14 = 0x0000000000000000
00:35:53     INFO -      r15 = 0x000000010efaae40   rip = 0x0000000102850183
00:35:53     INFO -      rsp = 0x00007fff5fbfd1c0   rbp = 0x00007fff5fbfd1f0
00:35:53     INFO -      Found by: given as instruction pointer in context
00:35:53     INFO -   1  XUL!ContinueCursorEvent::Run() [nsDeviceStorage.cpp:f28005b84ed0 : 1887 + 0x7]
00:35:53     INFO -      rbx = 0x00000001199a0400   r12 = 0x000000010059e8b0
00:35:53     INFO -      r13 = 0x00007fff5fbfd3c7   r14 = 0x0000000136c9dee0
00:35:53     INFO -      r15 = 0x000000010efaae40   rip = 0x0000000102850a61
00:35:53     INFO -      rsp = 0x00007fff5fbfd200   rbp = 0x00007fff5fbfd2d0
00:35:53     INFO -      Found by: call frame info
00:35:53     INFO -   2  XUL!nsThread::ProcessNextEvent(bool, bool*) [nsThread.cpp:f28005b84ed0 : 766 + 0x5]
00:35:53     INFO -      rbx = 0x000000010052faa0   r12 = 0x000000010052fa00
00:35:53     INFO -      r13 = 0x00007fff5fbfd3c7   r14 = 0x000000010052fb7c
00:35:53     INFO -      r15 = 0x000000010052fb7c   rip = 0x00000001014bd6fc
00:35:53     INFO -      rsp = 0x00007fff5fbfd2e0   rbp = 0x00007fff5fbfd3b0
00:35:53     INFO -      Found by: call frame info
00:35:53     INFO -   3  XUL!NS_ProcessPendingEvents(nsIThread*, unsigned int) [nsThreadUtils.cpp:f28005b84ed0 : 210 + 0xe]
00:35:53     INFO -      rbx = 0x000000010cd4e840   r12 = 0x000000010052faa0
00:35:53     INFO -      r13 = 0x00007fff5fbfd3c7   r14 = 0x0000000000000014
00:35:53     INFO -      r15 = 0x00000000000c3f13   rip = 0x000000010142834d
00:35:53     INFO -      rsp = 0x00007fff5fbfd3c0   rbp = 0x00007fff5fbfd3f0
00:35:53     INFO -      Found by: call frame info
00:35:53     INFO -   4  XUL!nsBaseAppShell::NativeEventCallback() [nsBaseAppShell.cpp:f28005b84ed0 : 98 + 0xe]
00:35:53     INFO -      rbx = 0x000000010cd4e840   r12 = 0x0000000000000000
00:35:53     INFO -      r13 = 0x0000000100115440   r14 = 0x000000010052faa0
00:35:53     INFO -      r15 = 0x000000010cd4e800   rip = 0x00000001025ccee7
00:35:53     INFO -      rsp = 0x00007fff5fbfd400   rbp = 0x00007fff5fbfd420
00:35:53     INFO -      Found by: call frame info
Summary: Intermittent test_usedSpace.html | application terminated with exit code 1 | application crashed [@ nsIFileToJsval(nsPIDOMWindow*, DeviceStorageFile*)] | Assertion failure: aWindow, at /builds/slave/m-cen-osx64-d-0000000000000000/build/dom/devicestorage/ → Intermittent test_usedSpace.html | application crashed [@ nsIFileToJsval(nsPIDOMWindow*, DeviceStorageFile*)] (after: "Assertion failure: aWindow, at nsDeviceStorage.cpp:1667")
Severity: normal → critical
Hardware: x86 → x86_64
Component: General → DOM: Device Interfaces
Assignee: nobody → xyuan
The cause of this bug:

At times the owner window of the DOMRequest is disconnected before we call the callback function from runnable. In that case, we pass null owner window to |nsIFileToJsval(nsPIDOMWindow*, DeviceStorageFile*)| and trigger an assertion error.


If the DOMRequest is disconnected and has null owner window, this patch avoids calling the callback function and prevents the assertion error.
Attachment #8450873 - Flags: review?(dhylands)
Push the above patch to try:
https://tbpl.mozilla.org/?tree=Try&rev=185d09204456
Status: NEW → ASSIGNED
Comment on attachment 8450873 [details] [diff] [review]
Check DOMRequest window before callback.

Review of attachment 8450873 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great - Thanks for digging into this.

Don't we need similar changes in DeviceStorageRequestChild.cpp?
In this function: http://dxr.mozilla.org/mozilla-central/source/dom/devicestorage/DeviceStorageRequestChild.cpp#54
Attachment #8450873 - Flags: review?(dhylands) → review+
(In reply to Dave Hylands [:dhylands] from comment #29)
> Comment on attachment 8450873 [details] [diff] [review]
> Check DOMRequest window before callback.
> 
> Review of attachment 8450873 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks great - Thanks for digging into this.
> 
> Don't we need similar changes in DeviceStorageRequestChild.cpp?
> In this function:
> http://dxr.mozilla.org/mozilla-central/source/dom/devicestorage/
> DeviceStorageRequestChild.cpp#54

Yes, we need it. I'll create another patch to fix DeviceStorageRequestChild.cpp.
Attached patch part2.patch (obsolete) — Splinter Review
Fix null owner window issue in DeviceStorageRequestChild.cpp.

I'll merge this patch into the previous one after review.
Attachment #8452096 - Flags: review?(dhylands)
Comment on attachment 8452096 [details] [diff] [review]
part2.patch

Review of attachment 8452096 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good - Thanks
Attachment #8452096 - Flags: review?(dhylands) → review+
Attached patch Gecko patchSplinter Review
Combine the two reviewed patch into one.
Attachment #8450873 - Attachment is obsolete: true
Attachment #8452096 - Attachment is obsolete: true
Attachment #8452207 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/55017488f655
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Please request Aurora/Beta/b2g30 approval on this patch when you get a chance.
Flags: needinfo?(xyuan)
OK, I will.
Flags: needinfo?(xyuan)
Comment on attachment 8452207 [details] [diff] [review]
Gecko patch

Approval Request Comment
[Feature/regressing bug #]: 
Bug 1019627 

[User impact if declined]: 
The debug build of gecko may crash when unloading a page that uses DeviceStorage API.

[Describe test coverage new/current, TBPL]: 
https://tbpl.mozilla.org/?tree=Try&rev=81fc4d8561ba

[Risks and why]:
No risk. The crash is caused by accessing null window pointer. The patch makes a check to avoid accessing null pointer and doesn't change the logic of the original code.

[String/UUID change made/needed]:
None.
Attachment #8452207 - Flags: approval-mozilla-beta?
Attachment #8452207 - Flags: approval-mozilla-b2g30?
Attachment #8452207 - Flags: approval-mozilla-aurora?
Attachment #8452207 - Flags: approval-mozilla-b2g30?
[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 1019627
User impact if declined: The debug build of gecko may crash when unloading a page that uses DeviceStorage API.
Testing completed: Tested on flame device and device storage worked fine.
Risk to taking this patch (and alternatives if risky): No risk.
String or UUID changes made by this patch: None.
Attachment #8453687 - Flags: review+
Attachment #8453687 - Flags: approval-mozilla-b2g30?
Yuan, It is not clear to me. Can this impact Firefox itself? Thanks
Flags: needinfo?(xyuan)
These tests are crashing on Fx desktop builds, if that answers your question.
Comment on attachment 8452207 [details] [diff] [review]
Gecko patch

That is enough to accept the patch. Thanks.
Attachment #8452207 - Flags: approval-mozilla-beta?
Attachment #8452207 - Flags: approval-mozilla-beta+
Attachment #8452207 - Flags: approval-mozilla-aurora?
Attachment #8452207 - Flags: approval-mozilla-aurora+
Flags: needinfo?(xyuan)
Comment on attachment 8453687 [details] [diff] [review]
Gecko patch for b2g30

This looks like a very safe stability fix. b2g30+
Attachment #8453687 - Flags: approval-mozilla-b2g30? → approval-mozilla-b2g30+
You need to log in before you can comment on or make changes to this bug.