Skip to content

inspector: overhaul JS binding implementation

Rodrigo Muino Tomonari requested to merge github/fork/TimothyGu/inspector into master

The first two commits of this PR are identical to https://github.com/nodejs/node/pull/14710. However, the last two commits fixed the GC problem pointed out by @bnoordhuis in https://github.com/nodejs/node/pull/14710#pullrequestreview-56313341, and makes Connection an AsyncWrap for async hooks support.

With

const sess = new inspector.Session();
sess.connect();
sess.post('Runtime.evaluate', {
  expression: 'Promise.resolve(3)',
  awaitPromise: true
}, (err, msg) => {
  console.log(msg);
});

and appropriate async hooks, whereas before this PR it looked like

PROMISE(4): trigger: 1 execution: 1
PROMISE(5): trigger: 4 execution: 1
PROMISE(6): trigger: 4 execution: 1
before:  5
{ result: { type: 'number', value: 3, description: '3' } }
  TickObject(7): trigger: 5 execution: 5
after:   5
before:  6
after:   6
before:  7
after:   7
destroy: 7

after it

INSPECTORJSBINDING(5): trigger: 1 execution: 1
PROMISE(6): trigger: 1 execution: 1
PROMISE(7): trigger: 6 execution: 1
PROMISE(8): trigger: 6 execution: 1
before:  7
  before:  5
{ result: { type: 'number', value: 3, description: '3' } }
    TickObject(9): trigger: 5 execution: 5
  after:   5
after:   7
before:  8
after:   8
before:  9
after:   9
destroy: 9

with proper instrumentation.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

inspector

Merge request reports

Loading