Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use callbacks instead of a promise that calls a callback for node:fs functions that expect callbacks #14109

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Jarred-Sumner
Copy link
Collaborator

What does this PR do?

This fixes a hypothetical execution-order bug in node:fs

Callbacks run before promises.

Since all the node:fs callback functions are currently implemented as wrappers around promises, that means we're delaying execution to the end of the current task. Since these are top-level tasks it should normally not make a difference, but in theory - maybe it could?

This works by making the API for callback-taking functions the same as promise-taking functions and then we don't need to care which is which most of the time

How did you verify your code works?

Untested, but wrote most of the code. Probably doesn't compile.

@robobun
Copy link

robobun commented Sep 23, 2024

@Jarred-Sumner, your commit e977421 has some failures in #3734

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants