1691661 - Remote target fronts can be frozen after calling TargetList stopListening

Status: NEW

(DevTools :: Framework, task, P3)

Description

[Julian Descottes [:jdescottes]]

This edge case occurred during the destroy of a mochitest after implementing Bug 1690698.

Here "frozen" front refers to a front on which you can still call actor methods, but the requests will never resolve. No exception will be thrown, nothing happens.

This happens only if certain conditions are met:

• there must be non-resources/non-targets data entries (eg breakpoints or configuration options)
• there must be a remote frame target

Under those conditions, calling targetList.stopListening will "freeze" all the fronts from the remote frame target.

Why is this happening

The reason the fronts become frozen is because we destroy the remote frame target in its process, as well as the transport, but we don't close the corresponding transport and connection in the parent process. See DevToolsFrameParent::destroyTarget, calling this method will completely destroy everything in the content process, but nothing is cleaned up in the parent process.

Usually this goes by unnoticed because quickly after that, we unregister the JS window actors, which will destroy all DevToolsFrameParent instances, which will close transports.

But here, if we have extra data-entry, such as breakpoints, we will not unregister the JS Window Actors. See WatcherRegistry::maybeUnregisteringJSWindowActor, as long as there's a breakpoint, the map entry for the watcher will remain, and we won't remove the js window actors.

Example during toolbox destroy

In the test browser_rules_media-queries.js, this happens when we destroy the toolbox and call targetList.destroy().

After that we will wait for the source-map service to finish its initialization.

But the source-map service needed to perform a call to getMediaRules on the stylesheet front for the remote frame (because it is watching StyleSheet resources, which use a legacy listener, which use the StyleSheet front etc....). And when the toolbox destroy happens really quickly, the call happens after we call stopListening and the stylesheet front is frozen, meaning getMediaRules never resolves and the toolbox destroy is blocked.

Next steps

This scenario highlights several potential defects, that might each be treated separately:

• maybe toolbox destroy should move the call to targetlist destroy after the sourcemap service is initialized (and maybe more reshuffling is needed)
• maybe the WatcherRegistry should unregister JSWindowActors if we are not watching for targets, and not care about resources/breakpoints/random-data-entry
• maybe the DevToolsFrameParent actor should cleanup the connection in the parent process when calling destroyTarget

Knowing that the target/toolbox destroy is a complicated mess, maybe we should also think about safety nets. Eg have a regular handshake at the transport layer to check that the other side is still responding, and if not, close the transport automatically.

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1691661 - Test showing frozen fronts after calling TargetList stopListening

Comment 2

[Julian Descottes [:jdescottes]]

The patch submitted here is NOT a fix, it's just a simple test that triggers the problem. The key part of the test is:

  // start listening, get remote frame target
  await targetList.startListening();
  const [, remoteFrameTarget] = targetList.getAllTargets([TargetList.TYPES.FRAME]);

  // watch frame targets, set a breapoint.
  await targetList.watchTargets([TargetList.TYPES.FRAME], onAvailable);
  const bp = await targetList.watcherFront.getBreakpointListActor();
  await bp.setBreakpoint({ line: 3, column: 2, sourceUrl: "http://some.url" });

  // unwatch frame targets, stop listening
  targetList.unwatchTargets([TargetList.TYPES.FRAME], onAvailable);
  targetList.stopListening();

  // use the remote frame target front
  await remoteFrameTarget.reconfigure({ options: {} });
  // This call should throw/return but it doesn't and the test freezes here.

Comment 3

[Chris Peterson [:cpeterson]]

dt-fission-future bugs don't need to block Fission MVP, so I'm moving them to Fission Future.