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

doc: fix up warning text about character devices #22569

Closed
wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

The text contained a number of inaccuracies:

  • The main thread is never blocked by a stream close.
  • There is no such thing as an EOF character on the OS level,
    the libuv level, or the Node.js stream level.
  • These streams do respond to .close(), but pending
    reads can delay this indefinitely.
  • Pushing a random character into the stream works only when
    the source data can be controlled; Using the JS .push()
    method is something different, and does not “unblock” any threads.

Refs: #21212

Pinging previous reviewers: @gireeshpunathil @vsemozhetbyt @jasnell @trivikr

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

The text contained a number of inaccuracies:

- The main thread is never blocked by a stream close.
- There is no such thing as an EOF character on the OS level,
  the libuv level, or the Node.js stream level.
- These streams *do* respond to `.close()`, but pending
  reads can delay this indefinitely.
- Pushing a random character into the stream works only when
  the source data can be controlled; Using the JS `.push()`
  method is something different, and does not “unblock” any threads.

Refs: nodejs#21212
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Aug 28, 2018
@gireeshpunathil
Copy link
Member

@addaleax - thanks for the rework. I am not fully aligned to your PR description, but the doc change is much more crisp and cleaner than mine, so I am good here. The inner details of what is happening at the thread and descriptor level is here: #15439 (comment) in case if you haven't already seen that.

@addaleax
Copy link
Member Author

@gireeshpunathil You can feel free to point out if I’m mistaken about anything. Regarding your linked comment:

As the file system work is carried out by internal worker thread, and it has initiated a blocking read from the device, and the data won't come, the main thread is unable to move ahead - it is held up in the polling for an async event that has to come from the said worker.

That issue pops up when we exhaust the thread pool, or when we try to exit the process (which involves waiting for all threads on POSIX systems), but before that the main thread should be unaffected?

@gireeshpunathil
Copy link
Member

this is how I visualized the issue:

  • a stream read is initiated. a worker thread is assigned to read.
  • as long as reads are happening, everything is fine.
  • when the device has no data, the worker thread blocks.
  • stream close has no effect as the fd in question is already engaged in reading.
  • when there is no other work pending (other than reading), loop thread invokes epoll_wait() with infinite timeout on the async handle pertinent to the async work. there is no way to alter this, so main thread blocks (this is not necessarily the exit route I guess)

EOF: character devices differ from block devices (such as files) in that the later issue a special character that causes the syscall.read() to yield. In char devices, the reader has to decide when to stop. When streams are abstracted around such devices, this mechanism is missing.

@addaleax
Copy link
Member Author

when there is no other work pending (other than reading), loop thread invokes epoll_wait() with infinite timeout on the async handle pertinent to the async work. there is no way to alter this, so main thread blocks (this is not necessarily the exit route I guess)

I guess that’s right, but we don’t usually refer to this as the main thread being blocked when it’s waiting for something to happen (but rather when it cannot get to the state of waiting for something to happen), do we?

And for the EOF character – that might be OS-dependent, but whether or not a special character is used on the hardware level is transparent to everything provided from the syscall level and upward, so it’s probably not a topic for our own docs?

@gireeshpunathil
Copy link
Member

agreed to both your points, thanks!

@addaleax
Copy link
Member Author

Landed in f2338ed

@addaleax addaleax closed this Aug 31, 2018
@addaleax addaleax deleted the fixup-fs-text branch August 31, 2018 17:16
addaleax added a commit that referenced this pull request Aug 31, 2018
The text contained a number of inaccuracies:

- The main thread is never blocked by a stream close.
- There is no such thing as an EOF character on the OS level,
  the libuv level, or the Node.js stream level.
- These streams *do* respond to `.close()`, but pending
  reads can delay this indefinitely.
- Pushing a random character into the stream works only when
  the source data can be controlled; Using the JS `.push()`
  method is something different, and does not “unblock” any threads.

Refs: #21212

PR-URL: #22569
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
addaleax added a commit that referenced this pull request Aug 31, 2018
The text contained a number of inaccuracies:

- The main thread is never blocked by a stream close.
- There is no such thing as an EOF character on the OS level,
  the libuv level, or the Node.js stream level.
- These streams *do* respond to `.close()`, but pending
  reads can delay this indefinitely.
- Pushing a random character into the stream works only when
  the source data can be controlled; Using the JS `.push()`
  method is something different, and does not “unblock” any threads.

Refs: #21212

PR-URL: #22569
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos pushed a commit that referenced this pull request Sep 3, 2018
The text contained a number of inaccuracies:

- The main thread is never blocked by a stream close.
- There is no such thing as an EOF character on the OS level,
  the libuv level, or the Node.js stream level.
- These streams *do* respond to `.close()`, but pending
  reads can delay this indefinitely.
- Pushing a random character into the stream works only when
  the source data can be controlled; Using the JS `.push()`
  method is something different, and does not “unblock” any threads.

Refs: #21212

PR-URL: #22569
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos pushed a commit that referenced this pull request Sep 6, 2018
The text contained a number of inaccuracies:

- The main thread is never blocked by a stream close.
- There is no such thing as an EOF character on the OS level,
  the libuv level, or the Node.js stream level.
- These streams *do* respond to `.close()`, but pending
  reads can delay this indefinitely.
- Pushing a random character into the stream works only when
  the source data can be controlled; Using the JS `.push()`
  method is something different, and does not “unblock” any threads.

Refs: #21212

PR-URL: #22569
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants