Test new features in more places

bindings/profilers/wall.cc

@@ -16,6 +16,7 @@
 
 #include <nan.h>
 #include <node.h>
+#include <v8-internal.h>
 #include <v8-profiler.h>
 #include <cinttypes>
 #include <cstdint>
@@ -41,6 +42,13 @@ struct TimeTicks {
   static int64_t Now();
 };
 }  // namespace base
+namespace internal {
+struct HandleScopeData {
+  v8::internal::Address* next;
+  v8::internal::Address* limit;
+};
+constexpr int kHandleBlockSize = v8::internal::KB - 2;
+}  // namespace internal
 }  // namespace v8
 
 static int64_t Now() {
@@ -1112,7 +1120,7 @@ v8::CpuProfiler* WallProfiler::CreateV8CpuProfiler() {
 }
 
 Local<Value> WallProfiler::GetContext(Isolate* isolate) {
-  auto context = GetContextPtr(isolate);
+  auto context = GetContextPtr(isolate, false);
   if (context) {
     return context->Get(isolate);
   }
@@ -1203,39 +1211,76 @@ ContextPtr WallProfiler::GetContextPtrSignalSafe(Isolate* isolate) {
     }
   }
 
-  return GetContextPtr(isolate);
+  return GetContextPtr(isolate, true);
+}
+
+v8::internal::HandleScopeData* getHandleScopeData(Isolate* isolate) {
+  return reinterpret_cast<v8::internal::HandleScopeData*>(
+      reinterpret_cast<uint64_t>(isolate) +
+      v8::internal::Internals::kIsolateHandleScopeDataOffset);
 }
 
-ContextPtr WallProfiler::GetContextPtr(Isolate* isolate) {
+// Returns the number of free Address slots for Locals that can be returned by
+// the isolate without triggering memory allocation.
+int GetFreeLocalSlotCount(Isolate* isolate) {
+  auto data = getHandleScopeData(isolate);
+  auto diff = data->limit - data->next;
+  // sanity check: diff can be at most kHandleBlockSize. If it is larger,
+  // something is suspicious. See
+  // https://github.com/v8/v8/blob/6fcfeccda2d8bcb7397f89bf5bbacd0c2eb2fb7f/src/handles/handles.cc#L195
+  return diff > v8::internal::kHandleBlockSize ? 0 : diff;
+}
+
+ContextPtr WallProfiler::GetContextPtr(Isolate* isolate, bool inSignalHandler) {
 #if NODE_MAJOR_VERSION >= 23
   if (!useCPED_) {
     return curContext_;
   }
 
-  if (!isolate->IsInUse()) {
-    // Must not try to create a handle scope if isolate is not in use.
-    return ContextPtr();
-  }
-  HandleScope scope(isolate);
-
-  auto cped = isolate->GetContinuationPreservedEmbedderData();
-  if (cped->IsObject()) {
-    auto v8Ctx = isolate->GetEnteredOrMicrotaskContext();
-    if (!v8Ctx.IsEmpty()) {
-      auto cpedObj = cped.As<Object>();
-      auto localSymbol = cpedSymbol_.Get(isolate);
-      auto maybeProfData = cpedObj->GetPrivate(v8Ctx, localSymbol);
-      if (!maybeProfData.IsEmpty()) {
-        auto profData = maybeProfData.ToLocalChecked();
-        if (!profData->IsUndefined()) {
-          return static_cast<PersistentContextPtr*>(
-                     profData.As<External>()->Value())
-              ->Get();
+  // HandleScope::CreateHandle() first increments HandleScopeData::next and only
+  // then writes to the Address slot in the source code, which is safe for us.
+  // The compiler is free to reorder these two operations, however, so we'll
+  // defensively save/restore the current value of the next Address slot in case
+  // the signal interrupted the thread between these two reordered operations.
+  v8::internal::Address* nextAddrPtr = nullptr;
+  v8::internal::Address savedAddr = 0;
+  if (inSignalHandler) {
+    if (!isolate->IsInUse() || GetFreeLocalSlotCount(isolate) < 4) {
+      // Must not try to create a handle scope if isolate is not in use or if we
+      // don't have at least 4 free slots to create local handles. The 4 handles
+      // are return values of GetCPED, GetEnteredOrMicrotaskContext,
+      // cpedSymbol_.Get, and GetPrivate.
+      return ContextPtr();
+    }
+    nextAddrPtr = getHandleScopeData(isolate)->next;
+    savedAddr = *nextAddrPtr;
+  }
+  ContextPtr retval = ContextPtr();
+  {
+    HandleScope scope(isolate);
+
+    auto cped = isolate->GetContinuationPreservedEmbedderData();
+    if (cped->IsObject()) {
+      auto v8Ctx = isolate->GetEnteredOrMicrotaskContext();
+      if (!v8Ctx.IsEmpty()) {
+        auto cpedObj = cped.As<Object>();
+        auto localSymbol = cpedSymbol_.Get(isolate);
+        auto maybeProfData = cpedObj->GetPrivate(v8Ctx, localSymbol);
+        if (!maybeProfData.IsEmpty()) {
+          auto profData = maybeProfData.ToLocalChecked();
+          if (!profData->IsUndefined()) {
+            retval = static_cast<PersistentContextPtr*>(
+                         profData.As<External>()->Value())
+                         ->Get();
+          }
         }
       }
     }
   }
-  return ContextPtr();
+  if (nextAddrPtr) {
+    *nextAddrPtr = savedAddr;
+  }
+  return retval;
 #else
   return curContext_;
 #endif
@@ -1271,8 +1316,10 @@ NAN_METHOD(WallProfiler::Dispose) {
 }
 
 double GetAsyncIdNoGC(v8::Isolate* isolate) {
-  if (!isolate->IsInUse()) {
-    // Must not try to create a handle scope if isolate is not in use.
+  if (!isolate->IsInUse() || GetFreeLocalSlotCount(isolate) < 1) {
+    // Must not try to create a handle scope if isolate is not in use or if
+    // we can't create one local handle (return value of
+    // GetEnteredOrMicrotaskContext) without allocation.
     return -1;
   }
 #if NODE_MAJOR_VERSION >= 24

bindings/profilers/wall.hh

@@ -114,7 +114,7 @@ class WallProfiler : public Nan::ObjectWrap {
   static void CleanupHook(void* data);
   void Cleanup(v8::Isolate* isolate);
 
-  ContextPtr GetContextPtr(v8::Isolate* isolate);
+  ContextPtr GetContextPtr(v8::Isolate* isolate, bool inSignalHandler);
   ContextPtr GetContextPtrSignalSafe(v8::Isolate* isolate);
 
   void SetCurrentContextPtr(v8::Isolate* isolate, v8::Local<v8::Value> context);

ts/test/test-time-profiler.ts

@@ -26,12 +26,14 @@ import {GenerateTimeLabelsArgs, LabelSet} from '../src/v8-types';
 import {AsyncLocalStorage} from 'async_hooks';
 import {satisfies} from 'semver';
 
-const assert = require('assert');
+import assert from 'assert';
 
 const useCPED =
   satisfies(process.versions.node, '>=24.0.0') &&
   !process.execArgv.includes('--no-async-context-frame');
 
+const collectAsyncId = satisfies(process.versions.node, '>=24.0.0');
+
 const PROFILE_OPTIONS = {
   durationMillis: 500,
   intervalMicros: 1000,
@@ -116,6 +118,7 @@ describe('Time Profiler', () => {
         intervalMicros: PROFILE_OPTIONS.intervalMicros,
         durationMillis: PROFILE_OPTIONS.durationMillis,
         withContexts: true,
+        collectAsyncId: collectAsyncId,
         lineNumbers: false,
         useCPED,
       });
@@ -125,6 +128,7 @@ describe('Time Profiler', () => {
       const rootSpanId = '1234';
       const endPointLabel = 'trace endpoint';
       const rootSpanIdLabel = 'local root span id';
+      const asyncIdLabel = 'async id';
       const endPoint = 'foo';
       let enableEndPoint = false;
       const label0 = {label: 'value0'};
@@ -136,7 +140,7 @@ describe('Time Profiler', () => {
         validateProfile(
           time.stop(
             i < repeats - 1,
-            enableEndPoint ? generateLabels : undefined
+            enableEndPoint || collectAsyncId ? generateLabels : undefined
           )
         );
       }
@@ -145,9 +149,11 @@ describe('Time Profiler', () => {
         if (!context) {
           return {};
         }
-        // Does not collect async IDs by default
-        assert(typeof context.asyncId === 'undefined');
         const labels: LabelSet = {};
+        if (typeof context.asyncId !== 'undefined') {
+          assert(collectAsyncId);
+          labels[asyncIdLabel] = context.asyncId;
+        }
         for (const [key, value] of Object.entries(context.context ?? {})) {
           if (typeof value === 'string') {
             labels[key] = value;
@@ -215,13 +221,16 @@ describe('Time Profiler', () => {
       function validateProfile(profile: Profile) {
         // Get string table indices for strings we're interested in
         const stringTable = profile.stringTable;
-        const [loopIdx, fn0Idx, fn1Idx, fn2Idx, hrtimeBigIntIdx] = [
-          'loop',
-          'fn0',
-          'fn1',
-          'fn2',
-          'hrtimeBigInt',
-        ].map(x => stringTable.dedup(x));
+        const [
+          loopIdx,
+          fn0Idx,
+          fn1Idx,
+          fn2Idx,
+          hrtimeBigIntIdx,
+          asyncIdLabelIdx,
+        ] = ['loop', 'fn0', 'fn1', 'fn2', 'hrtimeBigInt', asyncIdLabel].map(x =>
+          stringTable.dedup(x)
+        );
 
         function getString(n: number | bigint): string {
           if (typeof n === 'number') {
@@ -259,6 +268,7 @@ describe('Time Profiler', () => {
         let fn0ObservedWithLabel0 = false;
         let fn1ObservedWithLabel1 = false;
         let fn2ObservedWithoutLabels = false;
+        let observedAsyncId = false;
         profile.sample.forEach(sample => {
           let fnName;
           for (const locationId of sample.locationId) {
@@ -272,7 +282,17 @@ describe('Time Profiler', () => {
             }
           }
           const labels = sample.label;
-
+          if (collectAsyncId) {
+            const idx = labels.findIndex(
+              label => label.key === asyncIdLabelIdx
+            );
+            if (idx !== -1) {
+              // Remove async ID label so it doesn't confuse the assertions on
+              // labels further below.
+              labels.splice(idx, 1);
+              observedAsyncId = true;
+            }
+          }
           switch (fnName) {
             case loopIdx:
               if (enableEndPoint) {
@@ -366,12 +386,13 @@ describe('Time Profiler', () => {
           fn2ObservedWithoutLabels,
           'fn2 was not observed without a label'
         );
+        assert(!collectAsyncId || observedAsyncId, 'Async ID was not observed');
       }
     });
   });
 
   it('should have async IDs when enabled', async function shouldCollectAsyncIDs() {
-    if (process.platform !== 'darwin' && process.platform !== 'linux') {
+    if (!(collectAsyncId && ['darwin', 'linux'].includes(process.platform))) {
       this.skip();
     }
     this.timeout(3000);

ts/test/worker.ts

@@ -15,6 +15,8 @@ const withContexts =
 const useCPED =
   satisfies(process.versions.node, '>=24.0.0') &&
   !process.execArgv.includes('--no-async-context-frame');
+const collectAsyncId =
+  withContexts && satisfies(process.versions.node, '>=24.0.0');
 
 function createWorker(durationMs: number): Promise<Profile[]> {
   return new Promise((resolve, reject) => {
@@ -64,6 +66,7 @@ async function main(durationMs: number) {
     withContexts,
     collectCpuTime: withContexts,
     useCPED: useCPED,
+    collectAsyncId: collectAsyncId,
   });
   if (withContexts) {
     time.setContext({});
@@ -108,6 +111,7 @@ async function worker(durationMs: number) {
     withContexts,
     collectCpuTime: withContexts,
     useCPED: useCPED,
+    collectAsyncId: collectAsyncId,
   });
   if (withContexts) {
     time.setContext({});

ts/test/worker2.ts

@@ -1,5 +1,6 @@
 import {parentPort} from 'node:worker_threads';
 import {time} from '../src/index';
+import {satisfies} from 'semver';
 
 const delay = (ms: number) => new Promise(res => setTimeout(res, ms));
 
@@ -8,12 +9,20 @@ const INTERVAL_MICROS = 10000;
 const withContexts =
   process.platform === 'darwin' || process.platform === 'linux';
 
+const useCPED =
+  satisfies(process.versions.node, '>=24.0.0') &&
+  !process.execArgv.includes('--no-async-context-frame');
+
+const collectAsyncId =
+  withContexts && satisfies(process.versions.node, '>=24.0.0');
+
 time.start({
   durationMillis: DURATION_MILLIS,
   intervalMicros: INTERVAL_MICROS,
   withContexts: withContexts,
   collectCpuTime: withContexts,
-  collectAsyncId: false,
+  collectAsyncId: collectAsyncId,
+  useCPED: useCPED,
 });
 
 parentPort?.on('message', () => {