1062629 - [jsdbg2] Off-thread parsing always parses in a non-debug mode compartment

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Shu-yu Guo [:shu]]

The off-thread parse task compartment is always non-debug, even when the main thread compartment is in debug mode.

Comment 1

[Shu-yu Guo [:shu]]

Attachment: Off-thread compartment debug mode should match main thread compartment debug mode.

Comment 2

[Shu-yu Guo [:shu]]

Attachment: Off-thread compartment debug mode should match main thread compartment debug mode.

Comment 3

[Jim Blandy :jimb]

Comment on attachment 8483889 [details] [diff] [review]
Off-thread compartment debug mode should match main thread compartment debug mode.

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

I think this also needs a comment on JSCompartment::debugMode itself. Is this going to be simplified when debug mode doesn't inhibit optimizations, or complicated?

Comment 4

[Jim Blandy :jimb]

This should be ready to land, right?

Comment 5

[Jim Blandy :jimb]

Oh, it's blocked by bug 1032869. Gotcha.

Comment 6

[Jim Blandy :jimb]

Comment on attachment 8483889 [details] [diff] [review]
Off-thread compartment debug mode should match main thread compartment debug mode.

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

::: js/src/jscompartment.cpp
@@ +596,5 @@
>  
>      if (global_ && IsObjectAboutToBeFinalized(global_.unsafeGet())) {
> +        // For main thread compartments, the invariant is that debug mode
> +        // implies having at least one Debugger still attached. However, for
> +        // off-thread compartments (which are used in off-thread parsing,

This opening parenthesis is never closed.

Instead of this weird debugMode() and invisibility thing (which looks *weird*), why not just check global->getDebuggers().length? Isn't that exactly the condition we need?

Comment 7

[Shu-yu Guo [:shu]]

(In reply to Jim Blandy :jimb from comment #6)
> Comment on attachment 8483889 [details] [diff] [review]
> Off-thread compartment debug mode should match main thread compartment debug
> mode.
> 
> Review of attachment 8483889 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> 
> Instead of this weird debugMode() and invisibility thing (which looks
> *weird*), why not just check global->getDebuggers().length? Isn't that
> exactly the condition we need?

That's the bug: the off-thread compartment has its own global, and has no Debuggers, even though it's doing work on behalf of the main-thread compartment, which does have Debuggers.

Comment 8

[Shu-yu Guo [:shu]]

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

Comment 9

[Ekanan Ketunuti]

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