AsyncLocalStorage potential issue ? #33580

Activity

1. targos

   added

   async_hooksIssues and PRs related to the async hooks subsystem.Issues and PRs related to the async hooks subsystem.

   on May 27, 2020
2. vdeturckheim commented on May 27, 2020

   

   vdeturckheim

   on May 27, 2020

   Member

   More actions

   Hello @FranckFreiburger !

   that's a good point. I remember we discussed it with @Qard , @puzpuzpuz and @Flarna on the PR but I can't find it back. There might actually be a memory leak in case of a reused resource.
   I need to circle back in the discussions but this is a massive one and Github has a hard time with it(#26540).
3. Qard commented on May 27, 2020

   

   Qard

   on May 27, 2020

   Member

   More actions

   All existing resource reuse should be using an AsyncResource for each reuse, so that should no longer be true. It would be a bug if anything doesn't use AsyncResource when reusing a resource now.

   The line in the doc is not exactly true anymore, though I hesitate slightly to remove it as it's still possible for native modules to reuse a resource without an AsyncResource and there's not really anything we can do to stop them. I lean on the side of continuing to discourage people from relying on that capability.

   👍React with 👍2Reacted by Vladimir de Turckheim and Andrei Pechkurov
4. puzpuzpuz

   added

   questionIssues asking questions about Node.js.Issues asking questions about Node.js.

   docIssues and PRs related to Node.js documentation.Issues and PRs related to Node.js documentation.

   on May 28, 2020
5. Flarna commented on May 28, 2020

   

   Flarna

   on May 28, 2020

   Member

   More actions

   There should be no reuse anymore within node core - at least not in the sense of the resource visible via async hooks.
   HTTPParser objects are still reused and also the Socket in HTTP agent. But on every reuse a new "wrapper object" passed to AsyncHooks is created (even it's not of type AsyncResource).

   see e.g

   node/lib/_http_agent.js

   Line 235 in 5d81e4d

   handle.asyncReset(new ReusedHandle(handle.getProviderType(), handle));

   for resued socket handle in HTTP agent.

   But there is no way to gurantee that some userland module reuses an resource object. And this potential reuse can be native or managed.

   There is an open PR regarding potential wrong reuse via NAPI: #32930 But again it dependes on the actual native addon.

   👍React with 👍2Reacted by Andrei Pechkurov and Vladimir de Turckheim
6. puzpuzpuz commented on Jun 4, 2020

   

   puzpuzpuz

   on Jun 4, 2020

   Member

   More actions

   @FranckFreiburger
   As the question seems to be answered, is it ok to close the issue? Please feel free to submit a PR that would add more context to the doc.
7. FranckFreiburger

   closed this as completedon Jun 11, 2020