async_hooks: avoid resource reuse by FileHandle

Wrap reused read_wrap in a unique async resource to ensure that
executionAsyncResource() is not ambiguous.
PR-URL: #31972
Refs: #30959
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

Author: Flarna

src/async_wrap.cc

@@ -587,7 +587,7 @@ AsyncWrap::AsyncWrap(Environment* env,
  provider_type_ = provider;
 
 // Use AsyncReset() call to execute the init() callbacks.
-AsyncReset(execution_async_id, silent);
+AsyncReset(object, execution_async_id, silent);
  init_hook_ran_ = true;
 }
 
@@ -653,10 +653,6 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) {
  env->destroy_async_id_list()->push_back(async_id);
 }
 
-voidAsyncWrap::AsyncReset(double execution_async_id, bool silent) {
-AsyncReset(object(), execution_async_id, silent);
-}
-
 // Generalized call for both the constructor and for handles that are pooled
 // and reused over their lifetime. This way a new uid can be assigned when
 // the resource is pulled out of the pool and put back into use.

src/async_wrap.h

@@ -170,9 +170,6 @@ class AsyncWrap : public BaseObject {
 double execution_async_id = kInvalidAsyncId,
 bool silent = false);
 
-voidAsyncReset(double execution_async_id = kInvalidAsyncId,
-bool silent = false);
-
 // Only call these within a valid HandleScope.
  v8::MaybeLocal<v8::Value> MakeCallback(const v8::Local<v8::Function> cb,
 int argc,

src/node_file.cc

@@ -369,7 +369,12 @@ int FileHandle::ReadStart() {
 if (freelist.size() > 0) {
  read_wrap = std::move(freelist.back());
  freelist.pop_back();
- read_wrap->AsyncReset();
+// Use a fresh async resource.
+// Lifetime is ensured via AsyncWrap::resource_.
+ Local<Object> resource = Object::New(env()->isolate());
+ resource->Set(
+env()->context(), env()->handle_string(), read_wrap->object());
+ read_wrap->AsyncReset(resource);
  read_wrap->file_handle_ = this;
  } else {
  Local<Object> wrap_obj;

test/async-hooks/test-filehandle-no-reuse.js

@@ -0,0 +1,65 @@
+'use strict';
+
+constcommon=require('../common');
+constinitHooks=require('./init-hooks');
+const{ checkInvocations }=require('./hook-checks');
+constfixtures=require('../common/fixtures');
+if(!common.hasCrypto)
+common.skip('missing crypto');
+consthttp2=require('http2');
+constassert=require('assert');
+constfs=require('fs');
+
+// Checks that the async resource is not reused by FileHandle.
+// Test is based on parallel\test-http2-respond-file-fd.js.
+
+consthooks=initHooks();
+hooks.enable();
+
+const{
+HTTP2_HEADER_CONTENT_TYPE
+}=http2.constants;
+
+// Use large fixture to get several file operations.
+constfname=fixtures.path('person-large.jpg');
+constfd=fs.openSync(fname,'r');
+
+constserver=http2.createServer();
+server.on('stream',(stream)=>{
+stream.respondWithFD(fd,{
+[HTTP2_HEADER_CONTENT_TYPE]: 'text/plain'
+});
+});
+server.on('close',common.mustCall(()=>fs.closeSync(fd)));
+server.listen(0,()=>{
+constclient=http2.connect(`http://localhost:${server.address().port}`);
+constreq=client.request();
+
+req.on('response',common.mustCall());
+req.on('data',()=>{});
+req.on('end',common.mustCall(()=>{
+client.close();
+server.close();
+}));
+req.end();
+});
+
+process.on('exit',onExit);
+
+functiononExit(){
+hooks.disable();
+hooks.sanityCheck();
+constactivities=hooks.activities;
+
+// Verify both invocations
+constfsReqs=activities.filter((x)=>x.type==='FSREQCALLBACK');
+assert.ok(fsReqs.length>=2);
+
+checkInvocations(fsReqs[0],{init: 1,destroy: 1},'when process exits');
+checkInvocations(fsReqs[1],{init: 1,destroy: 1},'when process exits');
+
+// Verify reuse handle has been wrapped
+assert.ok(fsReqs[0].handle!==fsReqs[1].handle,'Resource reused');
+assert.ok(fsReqs[0].handle===fsReqs[1].handle.handle,
+'Resource not wrapped correctly');
+}