node icon indicating copy to clipboard operation
node copied to clipboard

Leverage `using`/`Symbol.dispose` to ensure resources are cleaned up before Node.js exits

Open brillout opened this issue 1 year ago • 12 comments

What is the problem this feature will solve?

Today, there isn't a way to guarantee resource cleanup. For example:

export function doSomeWork() {
    const path = ".some_temp_file";
    const file = fs.openSync(path, "w+");

    try {
        // use file...

        if (someCondition()) {
            // do some more work...
            return;
        }
    }
    finally {
        // Close the file and delete it.
        fs.closeSync(file);
        fs.unlinkSync(path);
    }
}

If the process exits between fs.openSync() and the finally code block, then the file isn't removed.

What is the feature you are proposing to solve the problem?

While a 100% guarantee isn't possible, leveraging the new using keyword, there is an opportunity to dramatically increase the probability of successful resource cleanup.

Is that on Node.js's radar?

What alternatives have you considered?

No response

brillout avatar Jul 07 '23 06:07 brillout

Is that on Node.js's radar?

You can see multiple pull request already working on it and most of them are merged.

The first iteration of Symbol.dispose is released on 20.4.0, but the using keyword requires v8 support. https://bugs.chromium.org/p/v8/issues/detail?id=13559 https://bugs.chromium.org/p/v8/issues/detail?id=13879

climba03003 avatar Jul 07 '23 07:07 climba03003

Very neat.

the using keyword requires v8 support.

Makes sense. A workaround is to use TypeScript 5.2 which supports using.

brillout avatar Jul 07 '23 07:07 brillout

Would such a cleanup work even when v8 crashes like in heap out of memory cases, e.g. when no JS execution is possible any more.

silverwind avatar Jul 07 '23 07:07 silverwind

Would such a cleanup work even when v8 crashes like in heap out of memory cases, e.g. when no JS execution is possible any more.

Probably not

MoLow avatar Jul 07 '23 09:07 MoLow

Would such a cleanup work even when v8 crashes like in heap out of memory cases, e.g. when no JS execution is possible any more.

Probably not

I guess it would if the cleanup is performed in C++. That would be a truly reliable way for cleanup as the process exit hook is not triggered on V8 crash, but I guess another way to declare the resources will be needed as JS can not execute anymore then unless v8 is restarted which I would not advice to do.

silverwind avatar Jul 07 '23 09:07 silverwind

Hey,

Great to see users are asking for this!

As others have mentioned this is "on the roadmap" and we started adding Symbol.dispose and Symbol.asyncDispose support to all APIs. You can using timers, readable streams, file handles and mock timers so far.

We've done this in collaboration with the TypeScript team and coordination with Babel so users in transpilers would be able to benefit from it before V8 lands the syntactic support.

When the process crashes, electricity goes down or another catastrophic failure happens you cannot rely on disposers running. It is effectively a safer/neater alternative for try... finally.

benjamingr avatar Jul 07 '23 11:07 benjamingr

When the process crashes, electricity goes down or another catastrophic failure happens you cannot rely on disposers running. It is effectively a safer/neater alternative for try... finally.

Is there a difference between v8 crashing and node crashing? Couldn't node still run cleanup before exiting in case of v8 crash?

silverwind avatar Jul 07 '23 11:07 silverwind

I'm wondering if we should ship a defer helper since you can absolutely await using the file in your case which would close it but you would have to wrap it or try/finally it in order to also unlink it.

benjamingr avatar Jul 07 '23 11:07 benjamingr

Is there a difference between v8 crashing and node crashing? Couldn't node still run cleanup before exiting in case of v8 crash?

My point is that users cannot rely on app-level cleanup code running in all cases since we (and any other platform) cannot guarantee that it will run. There are some cases where cleanup will run (e.g. unhandled rejection since it would propagate through the stack and disposers would run) and some cases where they won't (e.g. segfault).

benjamingr avatar Jul 07 '23 11:07 benjamingr

What APIs would you expect support in other than streams/files @brillout ?

benjamingr avatar Jul 09 '23 12:07 benjamingr

Thanks for the ping. My only use case so far is clear up of temporary files, but I'm sure there are other use cases that I can't think of right now.

(Unrelated but since you're Node.js member, I wonder whether the Node.js team can influence the outcome of https://github.com/npm/rfcs/issues/665? It's a really bad situation. It's causing massive pain on a daily basis to Node.js users. If we can find a solution that would significantly address the reputation of "JavaScript is a mess".)

brillout avatar Jul 10 '23 07:07 brillout

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

github-actions[bot] avatar Feb 08 '24 01:02 github-actions[bot]

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

github-actions[bot] avatar Mar 10 '24 01:03 github-actions[bot]

Are there any docs on this? Googling Symbol.dispose site:https://nodejs.org or using keyword site:https://nodejs.org doesn't lead to any relevant docs.

Does the latest Node.js version support (somehwat) guaranteed temporary file removal?

brillout avatar Mar 11 '24 20:03 brillout

These are the only references I could find, not much to go off, its basically so new that there are no docs about it: https://nodejs.org/api/timers.html#immediatesymboldispose https://nodejs.org/api/events.html#eventsaddabortlistenersignal-listener https://nodejs.org/api/child_process.html#subprocesssymboldispose https://github.com/search?q=repo%3Anodejs%2Fnode+symbol.dispose&type=code

luchillo17 avatar Apr 11 '24 08:04 luchillo17

@luchillo17 note that for some cases we expose Symbol.asyncDispose where appropriate https://nodejs.org/api/stream.html#readablesymbolasyncdispose

The main issue in terms of DX is that because we're a JS and not a TS runtime our docs are for JS and thus we don't have good examples until this lands in JS land.

benjamingr avatar Apr 11 '24 08:04 benjamingr

@benjamingr Worry not, I'm not gunning for this feature just yet, I know tc39 is in stage 3 draft, I just came here out of curiosity as someone was showing it in the context of a TS unit test for teardown logic.

luchillo17 avatar Apr 11 '24 08:04 luchillo17

Is there anything available for deleting temporary files? This would quite nice for both Vite and Vike. (Temporary files not being cleaned up is a common issue.)

brillout avatar Apr 11 '24 08:04 brillout

@brillout I maintain a package (tmp-promise) with a disposer pattern that could use an update but I'm not aware of anything in core.

I think we can probably ship a promises disposable version of mkdtemp if that's common enough

(also TIL about vike, happy 10000 to me I guess :))

benjamingr avatar Apr 11 '24 08:04 benjamingr

Neat. Yea, I guess it needs to be a core thing, so that Node.js knows it should apply the cleanup if, for example, the user terminates the process by hitting ctrl-c before the disposable resolves.

(Thank you, I'm glad Vike resonates with you :))

brillout avatar Apr 11 '24 11:04 brillout