917982 - threadLifetimePool should actually be the lifetime of the thread actor

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Right now we blow away the threadLifetimePool on every navigation (including refresh), but the RDP spec says it should have the same lifetime as the thread actor.

This is contributing to race conditions like bug 833618, and the reason why we are forced to put the breakpoint actors in the connection's pool, when they should really be in the threadLifetimePool. Plus, the thread-lifetime-promoted objects should still be alive and inspectable on navigation due to them still being alive via the bfcache.

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: bug-917982-threadlifetimepool.patch

Jim, this should be a pretty quick and easy review, we just talked about this on IRC.

https://tbpl.mozilla.org/?tree=Try&rev=20155e0ec587

Comment 2

[Jim Blandy :jimb]

Comment on attachment 8337156 [details] [diff] [review]
bug-917982-threadlifetimepool.patch

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

::: toolkit/devtools/server/actors/script.js
@@ +538,5 @@
>      }
> +    if (!aKeepThreadLifetimePool && this._threadLifetimePool) {
> +      this.conn.removeActorPool(this._threadLifetimePool);
> +      this._threadLifetimePool = null;
> +    }

I think it would be fine to actually just move the 'removeActorPool' call to ThreadActor.prototype.disconnect, and then leave this function mysterious-boolean-argument-free. (With a doc fix, obv.)

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

https://hg.mozilla.org/integration/fx-team/rev/96af68a913f4

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/96af68a913f4