Skip to content

Remove async_hooks Sensitive/low-level Embedder API #15572

Closed
@AndreasMadsen

Description

@AndreasMadsen
  • Version: master (7a95392)
  • Platform: all
  • Subsystem: async_hooks

As I mentioned a few months ago I'm not comfortable exposing the low-level async_hooks JS API. In the documentation PR #12953 we agreed to keep the low-level JS API undocumented. Some time has now passed and I think we are in a better position to discuss this.

The low-level JS API is quite large, thus I would like to separate the discussion into:

Note: There is some overlap between emitInit and setInitTriggerId in terms of initTriggerId. Hopefully, that won't be an issue.


Background

For implementers the JavaScript Embedding API is used to emit the init,
before, after and destroy events. There are currently two APIs for
doing this.

  • The high-level AsyncResource class API.
  • The low-level newUid, emitInit, emitBefore, emitAfter, emitDestroy. Sometimes this is called the Sensitive Embedder API, but sometimes that also captures triggerAsyncIdScope(asyncId, cb) which is a very different discussion. Thus I will refer to it as low-level JavaScript Embedder API.

This discussion is just about the newUid, emitInit, emitBefore, emitAfter, emitDestroy API.

A common purpose for the Embedder API is to allow queued jobs to not lose the causal context. An example of this is setTimeout (shown implementation is only conceptual).

const { newUid, initTriggerId, emitInit,
        emitBefore, emitAfter, emitDestroy } = require('async_hooks');


function Timeout(timeout) {
  this.timeout = timeout;
  this.ontimeout = null;
}

function setTimeout(callback, timeout) {
  const handle = new Timeout(timeout);
  const asyncId = newUid();
  const triggerAsyncId = initTriggerId();
  emitInit(asyncId, 'Timeout', triggerAsyncId, handle);
  handle.ontimeout = function () {
    emitBefore(asyncId, triggerAsyncId);
    callback();
    emitAfter(asyncId);
    emitDestroy(asyncId);
  };

  // This may emit an init event if a new request is created, but it can also
  // add the timeout to an existing request. When the request is completed
  // it will emit before and after around all the callbacks.
  // The operation will typically be implemented in C++.
  // function done(callbackList) {
  //   emitBefore(...)
  //   callbackList.forEach((callback) => callback());
  //   emitAfter(...)
  //   emitDestroy(...)
  // }
  addToQueue(handle);
}

Alternatively, this can be implemented using the high-level AsyncResource
class.

const { AsyncResource } = require('async_hooks');

class Timeout extends AsyncResource {
  constructor(timeout) {
    this.timeout = timeout;
    this.ontimeout = null;
    super('Timeout');
  }
}

function setTimeout(callback, timeout) {
  const handle = new Timeout(timeout);
  handle.ontimeout = function () {
    handle.emitBefore();
    callback();
    handle.emitAfter();
    handle.emitDestroy();
  };

  addToQueue(handle);
}

The low-level JavaScript Embedder API exists to:

  • To make it easier to add the embedding calls to existing code.
  • To solve an important issue with
    multiple inheritance, where the Timeout class would already inherit from
    a different class, that the implementer doesn't control.

Issues

The issues with the low-level JavaScript Embedder API are:

  • emitInit can be called twice with the same async_id.
  • emitDestroy can be called twice with the same async_id. Also possible
    with AsyncResource, but we could quite easily fix that.
  • The triggerAsyncId in emitBefore can be different than in emitInit.
    Definetly the biggest issue, since it is rather confusing that triggerAsyncId
    can be null in emitInit but not in emitBefore.
  • It is possible to hijack an existing async_id and emit before and after.
    Might be very theoretical.

Okay, so the problems are perhaps not super serious. But I would like to argue that the reasons for the low-level JavaScript Embedder API are not valid, as one can easily use AsyncResource in all cases.

In general restricting the public API to just AsyncResource will give us much more flexibility in state validation. And I think it will be prone to much fewer errors.

Solution

The simplest solution is to create a separate handle object that just
points to the actual handle object.

const { AsyncResource } = require('async_hooks');

function Timeout(timeout) {
  this.timeout = timeout;
  this.ontimeout = null;
}

class TimeoutResource extends AsyncResource {
  constructor(handle) {
    this.handle = handle;
    super('Timeout');
  }
}

function setTimeout(callback, timeout) {
  const handle = new Timeout(timeout);
  const resource = new TimeoutResource(hanlde);

  handle.ontimeout = function () {
    resource.emitBefore();
    callback();
    resource.emitAfter();
    resource.emitDestroy();
  };

  addToQueue(handle);
}

If creating of the extra resource object is somehow inconvenient, it is
possible to call the AsyncResource methods directly on the handle object.

const { AsyncResource } = require('async_hooks');

function Timeout(timeout) {
  this.timeout = timeout;
  this.ontimeout = null;
}

function setTimeout(callback, timeout) {
  const handle = new Timeout(timeout);
  AsyncResource.call(handle, 'Timeout');

  handle.ontimeout = function () {
    AsyncResource.prototype.emitBefore.call(handle);
    callback();
    AsyncResource.prototype.emitAfter.call(handle);
    AsyncResource.prototype.emitDestroy.call(handle);
  };

  addToQueue(handle);
}

The latter solution would require that we use ES5 type classes. Alternatively
we could add a static emitInit method to AsyncResource that can act as a constructor.

const { AsyncResource } = require('async_hooks');

function Timeout(timeout) {
  this.timeout = timeout;
  this.ontimeout = null;
}

function setTimeout(callback, timeout) {
  const handle = new Timeout(timeout);
  AsyncResource.emitInit(handle, 'Timeout');

  handle.ontimeout = function () {
    AsyncResource.emitBefore(handle);
    callback();
    AsyncResource.emitAfter(handle);
    AsyncResource.emitDestroy(handle);
  };

  addToQueue(handle);
}

Our AsyncResource implementation would just be:

class AsyncResource {
  constructor(name) { AsyncResource.emitInit(this, name); }
  static emitInit(self, name) {
    // current constructor code
  }

  emitBefore() { AsyncResource.emitBefore(this); }
  static emitBefore(self) {
    // current AsyncResource.prototype.emitBefore code
  }

  emitAfter() { AsyncResource.emitAfter(this); }
  static emitAfter(self) {
    // current AsyncResource.prototype.emitAfter code
  }

  emitDestroy() { AsyncResource.emitDestroy(this); }
  static emitDestroy(self) {
    // current AsyncResource.prototype.emitDestroy code
  }

  // ...
}

/cc @nodejs/async_hooks

Metadata

Metadata

Assignees

No one assigned

    Labels

    async_hooksIssues and PRs related to the async hooks subsystem.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions