-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
Clarify race condition in blocking vs non-blocking docs (fixes #8253) #8254
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
base: main
Are you sure you want to change the base?
Clarify race condition in blocking vs non-blocking docs (fixes #8253) #8254
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example's description was accurate prior to this change.
The issue referenced described an issue with the example code, so, if anything, the example itself should be changed, not its description.
|
Hi @avivkeller I've updated the PR with the revised example. Please let me know if any modifications are required. |
|
|
||
| There are some patterns that should be avoided when dealing with I/O. Let's look | ||
| at an example: | ||
| There are some patterns that should be avoided when dealing with I/O. Let's look at an example: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated change
| at an example: | ||
| There are some patterns that should be avoided when dealing with I/O. Let's look at an example: | ||
|
|
||
| ```js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example change isn't what we want.
We want:
operationWithCallback(() => { ... })
operationWithoutCallback()
Clarify Race Condition in Blocking vs Non-Blocking Docs
This PR updates the “Dangers of Mixing Blocking and Non-Blocking Code” section with a revised example to reliably demonstrate the danger of mixing blocking and non-blocking calls.
Tested on Windows 10 Home 22H2 with Node.js v22.20.0 and v25.0.0, where the original example showed inconsistent behavior due to Windows file handle persistence.
🧩 Original Issue Description
In the Overview of Blocking vs Non-Blocking article, the section “Dangers of Mixing Blocking and Non-Blocking Code” included this example:
The text stated: “In the above example, fs.unlinkSync() is likely to be run before fs.readFile(), which would delete file.md before it is actually read.” However, testing revealed that fs.readFile() often succeeded on Windows due to file handle behavior, contradicting the implied failure.
Changes Made
Retained the original description’s intent, clarifying the race condition without altering its core message.
Added a try-finally block for cleanup and a note about asynchronous output.
✅ Updated Documentation Text
The revised example:
Description: “In the above example, fs.unlinkSync() is run before the asynchronous fs.readFile() operation, which deletes temp.md before the read can occur. This results in a failure (e.g., ENOENT error), demonstrating the danger of mixing blocking and non-blocking calls without proper sequencing. The try-finally block ensures file management.”
The safe alternative remains:
References
Original issue context from https://nodejs.org/en/learn/asynchronous-work/overview-of-blocking-vs-non-blocking
Repository: https://github.com/nodejs/nodejs.org
fixes #8253