perf_hooks: fix function wrapped by timerify to work correctly

PR-URL: nodejs/node#43330 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: James M Snell <[email protected]>
noslate-project · Oct 10, 2022 · 633ccb1 · 633ccb1
1 parent fca668f
commit 633ccb1
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 15 deletions.
diff --git a/lib/internal/perf/timerify.js b/lib/internal/perf/timerify.js
@@ -20,10 +20,6 @@ const {
   isHistogram
 } = require('internal/histogram');
 
-const {
-  isConstructor,
-} = internalBinding('util');
-
 const {
   codes: {
     ERR_INVALID_ARG_TYPE,
@@ -68,14 +64,13 @@ function timerify(fn, options = {}) {
       histogram);
   }
 
-  const constructor = isConstructor(fn);
-
   function timerified(...args) {
+    const isConstructorCall = new.target !== undefined;
     const start = now();
-    const result = constructor ?
+    const result = isConstructorCall ?
       ReflectConstruct(fn, args, fn) :
       ReflectApply(fn, this, args);
-    if (!constructor && typeof result?.finally === 'function') {
+    if (!isConstructorCall && typeof result?.finally === 'function') {
       return result.finally(
         FunctionPrototypeBind(
           processComplete,

diff --git a/src/node_util.cc b/src/node_util.cc
@@ -289,11 +289,6 @@ static void GuessHandleType(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(OneByteString(env->isolate(), type));
 }
 
-static void IsConstructor(const FunctionCallbackInfo<Value>& args) {
-  CHECK(args[0]->IsFunction());
-  args.GetReturnValue().Set(args[0].As<v8::Function>()->IsConstructor());
-}
-
 static void ToUSVString(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   CHECK_GE(args.Length(), 2);
@@ -344,7 +339,6 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(WeakReference::IncRef);
   registry->Register(WeakReference::DecRef);
   registry->Register(GuessHandleType);
-  registry->Register(IsConstructor);
   registry->Register(ToUSVString);
 }
 
@@ -384,7 +378,6 @@ void Initialize(Local<Object> target,
   env->SetMethodNoSideEffect(target, "getConstructorName", GetConstructorName);
   env->SetMethodNoSideEffect(target, "getExternalValue", GetExternalValue);
   env->SetMethod(target, "sleep", Sleep);
-  env->SetMethodNoSideEffect(target, "isConstructor", IsConstructor);
 
   env->SetMethod(target, "arrayBufferViewHasBuffer", ArrayBufferViewHasBuffer);
   Local<Object> constants = Object::New(env->isolate());

diff --git a/test/parallel/test-performance-function.js b/test/parallel/test-performance-function.js
@@ -123,3 +123,22 @@ const {
     });
   });
 })().then(common.mustCall());
+
+// Regression tests for https://github.com/nodejs/node/issues/40623
+{
+  assert.strictEqual(performance.timerify(function func() {
+    return 1;
+  })(), 1);
+  assert.strictEqual(performance.timerify(function() {
+    return 1;
+  })(), 1);
+  assert.strictEqual(performance.timerify(() => {
+    return 1;
+  })(), 1);
+  class C {}
+  const wrap = performance.timerify(C);
+  assert.ok(new wrap() instanceof C);
+  assert.throws(() => wrap(), {
+    name: 'TypeError',
+  });
+}