937006 - "Hit MOZ_CRASH(Failed to get caller.)" using setTimeout on IndexedDB call

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Jesse Ruderman]

Attachment: testcase (crashes Firefox when loaded)

Hit MOZ_CRASH(Failed to get caller.) at dom/indexedDB/IDBRequest.cpp:281

Bug 933351 added this MOZ_CRASH line (actually, upgraded it from a warning).

Nightly: bp-4e09ae5c-0a5d-4c5d-8568-5c8ba2131111

Comment 1

[Jesse Ruderman]

Attachment: stack (gdb)

Comment 2

[Bobby Holley (:bholley)]

Looks like the assumptions in the patch from bug 933351 Were not valid. Andrea, can you take a look?

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Wait, a window is running a timeout... How is it possible that we can't dig out the file/line info from that?

Comment 4

[Andrea Marchesini [:baku]]

It crashes because of the .bind(indexedDB). In IDBRequest.cpp we call |nsJSUtils::GetCallingLocation()| that calls |JS_DescribeScriptedCaller()|. This function return false (error) if the callee is not a JS script. In this case it's not.

Comment 5

[Andrea Marchesini [:baku]]

Attachment: crash.patch

This patch adds a mochitest and replaces the crash with a warning.

Comment 6

[Bobby Holley (:bholley)]

Comment on attachment 8335261 [details] [diff] [review]
crash.patch

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

(In reply to Andrea Marchesini (:baku) from comment #4)
> It crashes because of the .bind(indexedDB). In IDBRequest.cpp we call
> |nsJSUtils::GetCallingLocation()| that calls |JS_DescribeScriptedCaller()|.
> This function return false (error) if the callee is not a JS script. In this
> case it's not.

Ahah! This was a case that was undefined in the spec until recently, when Boris, Hixie and I sorted it out. The new strategy is that callbacks (passed to timeouts and such) capture their incumbent script, and restore it onto the stack when they're invoked.

Please file a followup bug on turning this back into a MOZ_CRASH, and make it depend on bug 937317. r=bholley with that.

::: dom/indexedDB/test/test_bug937006.html
@@ +11,5 @@
> +  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
> +
> +<script>
> +
> +setTimeout(indexedDB.deleteDatabase.bind(indexedDB), 0, 'x');

Please add a comment here explaining what this is doing.

Comment 7

[Andrea Marchesini [:baku]]

Attachment: crash.patch

Comment 8

[Andrea Marchesini [:baku]]

https://tbpl.mozilla.org/?tree=Try&rev=1a81496572ca

Comment 9

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cb123997572c

Comment 10

[(PTO June 19-July 7) Ryan VanderMeulen]

Backed out for Desktop B2G mochitest orange.
https://hg.mozilla.org/integration/mozilla-inbound/rev/7903faa79325

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

Comment 11

[Andrea Marchesini [:baku]]

Attachment: crash.patch

https://hg.mozilla.org/integration/mozilla-inbound/rev/125b1813b9da

Comment 12

[Jan Varga [:janv]]

The test should have been just added to testing/mochitest/b2g-desktop.json

Comment 13

[Andrea Marchesini [:baku]]

I'll re-enable this mochitest in a followup

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/125b1813b9da