1717584 - Protocol should not return responses containing destroyed actors

Status: RESOLVED FIXED

(DevTools :: Framework, task, P3)

Description

[Julian Descottes [:jdescottes]]

This is coming from an edge case spotted when working on Bug 1716960.

After slightly changing the inspector initialization, I ran into the following scenario when testing browser_inspector_fission_switch_target.js, when navigating from the parent process page to a content process page:

1 - as the various inspector UI panels finish their initialization, we call PageStyle.getApplied for a given node

2 - on the server, PageStyle.getApplied starts collecting node actors for its response (it's the "inherited" part of a rule, which happens to be a NodeActor) https://searchfox.org/mozilla-central/rev/f351e19360729b351bfc7c1386d6e4ca4ea676e2/devtools/server/actors/page-style.js#582

3 - then it needs to perform some async logic

4 - in the meantime the navigation starts, and the WalkerActor (which is the parentPool for Node actors), releases all the node actors from the unloaded "frame" https://searchfox.org/mozilla-central/rev/f351e19360729b351bfc7c1386d6e4ca4ea676e2/devtools/server/actors/inspector/walker.js#2533 . Note that this happens outside of the TargetActor destruction (in practice, it occurs on will-navigate), because this code is supposed to handle both top-level and sub frames.

5 - the node actors are destroyed

6 - PageStyle.getApplied has finished its async processing and tries to return the response, but the node actors collected earlier are now destroyed

7 - protocol.js will attempt to write the response and will see that it contains Actor instances without any actorID. It will therefore ask the PageStyle actor to manage them, reassign them a new actor id, and add them to PageStyle actor's pool.

8 - finally the TargetActor will soon after be destroyed, which will destroy the PageStyle actor via the Pool destroy mechanism. PageStyle will in turn try to destroy the "ghost" NodeActors it just started to manage. Those will crash, which will break the whole destroy sequence and later on prevents a proper target switching

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1717584 - [devtools] Protocol should not return responses containing destroyed actors

Depends on D118469

Returning a destroyed actor causes the actor to be re-added in a new pool and potentially be destroyed a second time.
Depending on how the actor class is implemented, it can break the destroy sequence on the server.

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1717584 - [devtools] Do not reuse destroyed frame actors in Thread actor

Depends on D118470

The ThreadActor is storing frame actors in a local map, on top of the regular pools managed by the framework.
If a frame corresponding to an already destroyed actor needs to be handled and returned again by the thread actor, it should create a completely new Actor instead of attempting to reuse an already destroyed object.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/326d28d0cc3f
[devtools] Protocol should not return responses containing destroyed actors r=ochameau
https://hg.mozilla.org/integration/autoland/rev/ff8225deba22
[devtools] Do not reuse destroyed frame actors in Thread actor r=ochameau

Comment 4

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/326d28d0cc3f
https://hg.mozilla.org/mozilla-central/rev/ff8225deba22