1562614 - Some protocol.js actors are overriding destroy and not calling Actor.destroy()

Status: NEW

(DevTools :: General, task, P2)

Description

[Julian Descottes [:jdescottes]]

Had a quick look at our protocol.js actors, and some of them override the destroy() method without calling the parent class' destroy method:

• StylesheetActor from stylesheets.js (link)
• SourceActor from source.js (link)
• FrameActor from frame.js (link)
• EnvironmentActor from environment.js (link)
• WebExtensionActor from webextension.js (link)

The Actor's destroy is:

  destroy() {
    super.destroy();
    this.actorID = null;
  }

which calls Pool's destroy:

  destroy() {
    const parent = this.parent();
    if (parent) {
      parent.unmanage(this);
    }
    if (!this.__poolMap) {
      return;
    }
    for (const actor of this.__poolMap.values()) {
      // Self-owned actors are ok, but don't need destroying twice.
      if (actor === this) {
        continue;
      }
      const destroy = actor.destroy;
      if (destroy) {
        // Disconnect destroy while we're destroying in case of (misbehaving)
        // circular ownership.
        actor.destroy = null;
        destroy.call(actor);
        actor.destroy = destroy;
      }
    }
    this.conn.removeActorPool(this, true);
    this.__poolMap.clear();
    this.__poolMap = null;
  }

Comment 1

[Alexandre Poirot [:ochameau]]

Nice find!

Looks like we need a typed language here ;)

I'm wondering if, similarly to pending request assertions, we could have "not-removed" actors assertion at some point in our destruction codepath?