Should finalizers run on exit in Node.js?
benjamingr opened this issue · 11 comments
Discussion was raised where in Java finalizers run on process exit.
In discussion people said they don't want people to put complicated logic in finalizers - so that might be a consideration.
Also really easy to do from the node side (with a process exit handler)
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.
I agree. Do not run them on exit.
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.
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.
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 🙂
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.
GC is inherently unreliable/ambiguous.
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.
Couldn't one argue that GC is fairly unambiguous when every value is about to go out of scope?
Except for resurrection bugs, which the weakref design took pains to avoid.
const [z,wr] = (() => {
function x() {}
const wr = new WeakRef(x);
function z() { return void x; }
return [z, wr];
})();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.
I think we have a solid answer to this question ("no"), so closing the issue.
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.