You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository was archived by the owner on Jan 25, 2022. It is now read-only.
Repository navigation
This repository was archived by the owner on Jan 25, 2022. It is now read-only.
I'd argue, we should not run finalizers on exit in general. On the web, they will most likely not be run on page exit. Finalizers are unreliable, and trying to give them guarantees about being called on process exit might lead people to depend on them more than they should. Finalizers shouldn't be used to release external resources. If we leave them uncalled in this case, it's more likely that small tests will run into the issue, leading the bugs to get fixed.
Reacted by Benjamin Gruenbaum, Sathya Gunasekaran, Mark S. Miller, Mathieu Hofman, Trotyl Yu and Andy Wingo
I don't have a strong opinion one way or the other, just asked if it would be possible since I know there is the option in Java. The thought I'd add is that it would be much easier to add it later if there ends up being a compelling case versus trying to remove it later on. For that reason, I think leaving it out to start is a reasonable way to go.
Reacted by Benjamin Gruenbaum and Daniel Ehrenberg
Note that doing this would almost certainly make shutdown significantly slower. I don't know much about v8's GC architecture, but would be surprised if it had to always do a full GC if an isolate is discarded. Doing that would be the only way to reliably run finalizers on exit.
What's more, you'd have to run multiple GCs until no more finalizers are run, because those might make additional objects unreachable.
Reacted by Daniel Ehrenberg, Sathya Gunasekaran, Mathieu Hofman and Sergey Rubanov
I would like to bring Agents into this discussion. An Agent is defined as having its own thread. With these Agents, the exit of an Agent does not coincide with the exit of the process. So, at least in the case of native data being attached to JavaScript objects, if finalizers are not run on Agent exit, there will potentially be leaks as Agents are fired up and discarded.
Now, in the browser, attaching native data to JS objects is always under the control of the browser itself so the maintainers of the browser can decide amongst themselves what mechanism to use for cleaning up the native data associated with an Agent when an Agent exits.
In contrast, in Node.js we have an open architecture where native addons can associate native data with JavaScript objects where we have no control over the nature of that data. Thus, we have to provide a standard way of informing native addons that this data needs to be freed. Until the advent of worker threads, this was pretty much beyond the scope of the project, because the native data was always freed along with the whole segment at process exit.
I guess the question is this: shall we go ahead and work on our own, Node.js standard way of informing native addons that the Agent (in our case the worker thread's node::Environment) is about to exit, or can this mechanism fit within the scope of the specification of the life cycle of an Agent? If the latter, then a second question arises: can existing finalization mechanisms, such as V8's weak callbacks, be (re-)?used in this case?
I doubt that any such agent-based cleanup should be tied to weakrefs. GC is inherently unreliable/ambiguous. Agent death is clear. Any resources held by an agent should not be handled by code within the agent, because that would imply that the agent isn't dead yet.
We should also think about what the failure unit for preemptive termination, and for enabling others to cleanup after such a unit terminates.
Erlang has a good model for cleanup following process death. Their process corresponds well to our agent. However, our shared array buffers (SABs) means that our agents are not shared-nothing units, and therefore are not separable re preemptive failure. If we only treat the agent cluster as the unit of preemptive failure, then we should think about agent-cluster death, and should seek to put cleanup logic in other agent clusters.
In any case, I think non of this is relevant to weakrefs.
Couldn't one argue that GC is fairly unambiguous when every value is about to go out of scope?
Any resources held by an agent should not be handled by code within the agent, because that would imply that the agent isn't dead yet.
So, it sounds like the entity that fired up the Agent could be made to notify of the Agent's impending doom before actually initiating such doom, but that's outside the scope of the Agent.
Let's say that z lasts until the agent as a whole is truly dead. We can see that z does not actually use x. However, depending on the representation the compiler chooses, the runtime may not be able to know that z will not use x. Say z is used during the cleanup of something else, which would happen on agent exit. At what point can we say that the agent has exited "enough" that x will no longer be used, so we can do cleanup within the agent prior to the agent exiting? I'm sure the answer is that we cannot do so reliably.
We can only cleanup an agent death reliably from outside the agent, after the agent itself is already dead.
FWIW I understand that the general pattern here is to use an auxiliary cleanup set.
Assuming all holdings are unique, when you register an external resource with a FinalizationGroup, you also add the resource's holding to an agent-global cleanup set.
When the FinalizationGroup's callback runs, it cleans up the resource and removes the holding from the cleanup set.
When the process exits, you explicitly clean up any remaining holdings in the cleanup set.
So, IIUC Node can implement agent-death cleanup itself.
Activity
benjamingr commentedon May 31, 2019
Also really easy to do from the node side (with a process exit handler)
littledan commentedon May 31, 2019
I'd argue, we should not run finalizers on exit in general. On the web, they will most likely not be run on page exit. Finalizers are unreliable, and trying to give them guarantees about being called on process exit might lead people to depend on them more than they should. Finalizers shouldn't be used to release external resources. If we leave them uncalled in this case, it's more likely that small tests will run into the issue, leading the bugs to get fixed.
erights commentedon May 31, 2019
I agree. Do not run them on exit.
mhdawson commentedon Jun 2, 2019
I don't have a strong opinion one way or the other, just asked if it would be possible since I know there is the option in Java. The thought I'd add is that it would be much easier to add it later if there ends up being a compelling case versus trying to remove it later on. For that reason, I think leaving it out to start is a reasonable way to go.
tschneidereit commentedon Jun 4, 2019
Note that doing this would almost certainly make shutdown significantly slower. I don't know much about v8's GC architecture, but would be surprised if it had to always do a full GC if an isolate is discarded. Doing that would be the only way to reliably run finalizers on exit.
What's more, you'd have to run multiple GCs until no more finalizers are run, because those might make additional objects unreachable.
gabrielschulhof commentedon Jul 8, 2019
I would like to bring Agents into this discussion. An Agent is defined as having its own thread. With these Agents, the exit of an Agent does not coincide with the exit of the process. So, at least in the case of native data being attached to JavaScript objects, if finalizers are not run on Agent exit, there will potentially be leaks as Agents are fired up and discarded.
Now, in the browser, attaching native data to JS objects is always under the control of the browser itself so the maintainers of the browser can decide amongst themselves what mechanism to use for cleaning up the native data associated with an Agent when an Agent exits.
In contrast, in Node.js we have an open architecture where native addons can associate native data with JavaScript objects where we have no control over the nature of that data. Thus, we have to provide a standard way of informing native addons that this data needs to be freed. Until the advent of worker threads, this was pretty much beyond the scope of the project, because the native data was always freed along with the whole segment at process exit.
I guess the question is this: shall we go ahead and work on our own, Node.js standard way of informing native addons that the Agent (in our case the worker thread's
node::Environment) is about to exit, or can this mechanism fit within the scope of the specification of the life cycle of an Agent? If the latter, then a second question arises: can existing finalization mechanisms, such as V8's weak callbacks, be (re-)?used in this case?@addaleax, given your comment, please help me out if I've missed anything 🙂
erights commentedon Jul 8, 2019
I doubt that any such agent-based cleanup should be tied to weakrefs. GC is inherently unreliable/ambiguous. Agent death is clear. Any resources held by an agent should not be handled by code within the agent, because that would imply that the agent isn't dead yet.
We should also think about what the failure unit for preemptive termination, and for enabling others to cleanup after such a unit terminates.
Erlang has a good model for cleanup following process death. Their process corresponds well to our agent. However, our shared array buffers (SABs) means that our agents are not shared-nothing units, and therefore are not separable re preemptive failure. If we only treat the agent cluster as the unit of preemptive failure, then we should think about agent-cluster death, and should seek to put cleanup logic in other agent clusters.
In any case, I think non of this is relevant to weakrefs.
gabrielschulhof commentedon Jul 8, 2019
Couldn't one argue that GC is fairly unambiguous when every value is about to go out of scope?
So, it sounds like the entity that fired up the Agent could be made to notify of the Agent's impending doom before actually initiating such doom, but that's outside the scope of the Agent.
erights commentedon Jul 8, 2019
Except for resurrection bugs, which the weakref design took pains to avoid.
Let's say that
zlasts until the agent as a whole is truly dead. We can see thatzdoes not actually usex. However, depending on the representation the compiler chooses, the runtime may not be able to know thatzwill not usex. Sayzis used during the cleanup of something else, which would happen on agent exit. At what point can we say that the agent has exited "enough" thatxwill no longer be used, so we can do cleanup within the agent prior to the agent exiting? I'm sure the answer is that we cannot do so reliably.We can only cleanup an agent death reliably from outside the agent, after the agent itself is already dead.
littledan commentedon Sep 11, 2019
I think we have a solid answer to this question ("no"), so closing the issue.
wingo commentedon Sep 27, 2019
FWIW I understand that the general pattern here is to use an auxiliary cleanup set.
Assuming all holdings are unique, when you register an external resource with a FinalizationGroup, you also add the resource's holding to an agent-global cleanup set.
When the FinalizationGroup's callback runs, it cleans up the resource and removes the holding from the cleanup set.
When the process exits, you explicitly clean up any remaining holdings in the cleanup set.
So, IIUC Node can implement agent-death cleanup itself.
src: render N-API weak callbacks as cleanup hooks
src: render N-API weak callbacks as cleanup hooks