https://github.com/nodejs/node/blob/9949a2e1e3100c4ff1f228bac57c1af95cdc3e9d/lib/async_hooks.js#L248
You are using the resource object to store the store whereas the doc state :
In some cases the resource object is reused for performance reasons, it is thus not safe to use it as a key in a WeakMap or add properties to it.
I just wondering if it is ok.
FranckFreiburger
All 4 comments
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(https://github.com/nodejs/node/pull/26540).
vdeturckheim
on 27 May 2020
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.
Qard
on 28 May 2020
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 https://github.com/nodejs/node/blob/5d81e4d677e8a00c2d0d3a23e2bd74bf9f1deb57/lib/_http_agent.js#L235 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: https://github.com/nodejs/node/pull/32930 But again it dependes on the actual native addon.
Flarna
on 28 May 2020
@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.
puzpuzpuz
on 4 Jun 2020
Related issues
jonathanong
路
91Comments
TazmanianDI
路
127Comments
benjamingr
路
135Comments
egoroof
路
90Comments
ctavan
路
87Comments
Most helpful comment
There should be no reuse anymore within node core - at least not in the sense of the resource visible via async hooks.
HTTPParserobjects are still reused and also theSocketin HTTP agent. But on every reuse a new "wrapper object" passed to AsyncHooks is created (even it's not of typeAsyncResource).see e.g https://github.com/nodejs/node/blob/5d81e4d677e8a00c2d0d3a23e2bd74bf9f1deb57/lib/_http_agent.js#L235 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: https://github.com/nodejs/node/pull/32930 But again it dependes on the actual native addon.