1282113 - Crashes involving setGCCallback or Assertion failure: typeDescrObjects.empty(), at js/src/gc/Zone.cpp:61

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

The following testcase crashes on mozilla-central revision 939ecc4e9d05 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --no-baseline --no-ion):

Object.getOwnPropertyNames(this);
setGCCallback({
    action: "majorGC",
    phases: "begin"
});
selectforgc(this);


Backtrace:

0   js-dbg-64-dm-clang-darwin-939ecc4e9d05	0x000000010a1fcdce JS::Zone::~Zone() + 462 (Zone.cpp:61)
1   js-dbg-64-dm-clang-darwin-939ecc4e9d05	0x0000000109d44997 js::gc::GCRuntime::finish() + 391 (Utility.h:256)
2   js-dbg-64-dm-clang-darwin-939ecc4e9d05	0x0000000109f80878 JSRuntime::~JSRuntime() + 1032 (Runtime.cpp:467)
3   js-dbg-64-dm-clang-darwin-939ecc4e9d05	0x0000000109c8b156 JS_DestroyRuntime(JSRuntime*) + 22 (Utility.h:256)
4   js-dbg-64-dm-clang-darwin-939ecc4e9d05	0x00000001096ec181 main + 13249 (js.cpp:7453)
5   js-dbg-64-dm-clang-darwin-939ecc4e9d05	0x00000001096e6934 start + 52
/snip

For detailed crash information, see attachment.

Setting [fuzzblocker] because this is happening frequently and also s-s as a start because GC is in the testcase.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: Detailed Crash Information

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20160622003502" and the hash "e3c00a0692367cc1ea6cf6f40813abfaa74eba8a".
The "bad" changeset has the timestamp "20160622005003" and the hash "0ca871e39a20d94c5c8948beb41867d679f3709e".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e3c00a0692367cc1ea6cf6f40813abfaa74eba8a&tochange=0ca871e39a20d94c5c8948beb41867d679f3709e

Jan, is bug 1279295 a likely regressor?

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Basically, setGCCallback is virtually broken as it is causing various assertion failures and crashes including this one.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Patch

We no longer do the DESTROY_CONTEXT GC, I think that exposed this somehow.

While we do a DESTROY_RUNTIME GC, we trigger the GC callback and it starts a major GC. This nested GC then resets some global state, so the DESTROY_RUNTIME GC no longer cleans up everything.

This patch disallows non-shutdown GCs while we're destroying the runtime. We're going to destroy everything anyway.

Comment 5

[Jan de Mooij [:jandem]]

See also bug 1277833, an intermittent with the same signature. It started 3 weeks ago so it's definitely unrelated to bug 1279295.

I wonder if it could be related.

Comment 6

[Terrence Cole [:terrence]]

Comment on attachment 8765369 [details] [diff] [review]
Patch

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

::: js/public/GCAPI.h
@@ -53,5 @@
>      /* Reasons internal to the JS engine */     \
>      D(API)                                      \
>      D(EAGER_ALLOC_TRIGGER)                      \
>      D(DESTROY_RUNTIME)                          \
> -    D(DESTROY_CONTEXT)                          \

If you just delete this, it will throw off all of the reason telemetry buckets. Please instead replace D(DESTROY_CONTEXT) with D(UNUSED0).

::: js/src/jsgc.cpp
@@ +6282,5 @@
>  
> +    // Only allow shutdown GCs when we're destroying the runtime. This keeps
> +    // the GC callback from triggering a nested GC and resetting global state.
> +    if (rt->isBeingDestroyed() && !IsShutdownGC(reason))
> +        return false;

Ugh. It's absolutely awful that GC an recurse at all, but that's the status quo right now.

Comment 7

[Jan de Mooij [:jandem]]

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

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bc6e009a06d2

Comment 9

[Fuzzing Team]

JSBugMon: This bug has been automatically verified fixed.