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.

Sign up for GitHub

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

Jump to bottom

src: don't abort on EIO when restoring tty #28490

Closed
wants to merge 1 commit into from
Closed

src: don't abort on EIO when restoring tty #28490

wants to merge 1 commit into from

Conversation

bnoordhuis
Copy link
Member

EIO has been observed to be returned by the Linux kernel under some
circumstances. Reading through drivers/tty/tty_io*.c, it seems to
indicate the tty went away. Of course none of this is documented.

Fixes: #28479

@bnoordhuis
src: don't abort on EIO when restoring tty
1ba286c 
EIO has been observed to be returned by the Linux kernel under some
circumstances. Reading through drivers/tty/tty_io*.c, it seems to
indicate the tty went away. Of course none of this is documented.

Fixes: nodejs#28479
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jul 1, 2019
@bnoordhuis bnoordhuis mentioned this pull request Jul 1, 2019
Closed
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/24182/

@targos
Copy link
Member

targos commented Jul 1, 2019

Do we want to fast track? I'm going to prepare a release tomorrow

@addaleax addaleax added the fast-track PRs that do not need to wait for 48 hours to land. label Jul 1, 2019
@addaleax addaleax added tty Issues and PRs related to the tty subsystem. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jul 1, 2019
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/24184/

@devsnek
Copy link
Member

devsnek commented Jul 2, 2019

landed in 1095635

@devsnek devsnek closed this Jul 2, 2019
devsnek pushed a commit that referenced this pull request Jul 2, 2019
@bnoordhuis @devsnek
src: don't abort on EIO when restoring tty
1095635 
EIO has been observed to be returned by the Linux kernel under some
circumstances. Reading through drivers/tty/tty_io*.c, it seems to
indicate the tty went away. Of course none of this is documented.

Fixes: #28479

PR-URL: #28490
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
@devsnek devsnek mentioned this pull request Jul 2, 2019
Closed
targos pushed a commit that referenced this pull request Jul 2, 2019
@bnoordhuis @targos
src: don't abort on EIO when restoring tty
a0638b0 
EIO has been observed to be returned by the Linux kernel under some
circumstances. Reading through drivers/tty/tty_io*.c, it seems to
indicate the tty went away. Of course none of this is documented.

Fixes: #28479

PR-URL: #28490
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
targos pushed a commit that referenced this pull request Jul 2, 2019
@bnoordhuis @targos
src: don't abort on EIO when restoring tty
035b613 
EIO has been observed to be returned by the Linux kernel under some
circumstances. Reading through drivers/tty/tty_io*.c, it seems to
indicate the tty went away. Of course none of this is documented.

Fixes: #28479

PR-URL: #28490
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
targos added a commit that referenced this pull request Jul 2, 2019
@targos
2019-07-03, Version 12.6.0 (Current)
d711542 
Notable changes:

* build:
  * Experimental support for building Node.js on MIPS architecture
    is back. #27992
* child_process:
  * The promisified versions of `child_process.exec` and
    `child_process.execFile` now both return a `Promise` which has the
	child instance attached to their `child` property.
	#28325
* deps:
  * Updated libuv to 1.30.0. #28449
    * Support for the Haiku platform has been added.
    * The maximum `UV_THREADPOOL_SIZE` has been increased from 128 to
	  1024.
    * `uv_fs_copyfile()` now works properly when the source and
	  destination files are the same.
* process:
  * A new method, `process.resourceUsage()` was added. It returns
    resource usage for the current process, such as CPU time.
	#28018
* src:
  * Fixed an issue related to stdio that could lead to a crash of the
    process in some circumstances.
	#28490
* stream:
  * Added a `writableFinished` property to writable streams. It
    indicates that all the data has been flushed to the underlying
	system. #28007
* worker:
  * Fixed an issue that prevented worker threads to listen for data on
    stdin. #28153

PR-URL: #28508
@targos targos mentioned this pull request Jul 2, 2019
Merged
targos added a commit that referenced this pull request Jul 2, 2019
@targos
2019-07-03, Version 12.6.0 (Current)
e6a8ac1 
Notable changes:

* build:
  * Experimental support for building Node.js on MIPS architecture
    is back. #27992
* child_process:
  * The promisified versions of `child_process.exec` and
    `child_process.execFile` now both return a `Promise` which has the
	child instance attached to their `child` property.
	#28325
* deps:
  * Updated libuv to 1.30.0. #28449
    * Support for the Haiku platform has been added.
    * The maximum `UV_THREADPOOL_SIZE` has been increased from 128 to
	  1024.
    * `uv_fs_copyfile()` now works properly when the source and
	  destination files are the same.
* process:
  * A new method, `process.resourceUsage()` was added. It returns
    resource usage for the current process, such as CPU time.
	#28018
* src:
  * Fixed an issue related to stdio that could lead to a crash of the
    process in some circumstances.
	#28490
* stream:
  * Added a `writableFinished` property to writable streams. It
    indicates that all the data has been flushed to the underlying
	system. #28007
* worker:
  * Fixed an issue that prevented worker threads to listen for data on
    stdin. #28153
* meta:
  * Added Jiawen Geng (https://github.com/gengjiawen) to collaborators.
    #28322

PR-URL: #28508
targos added a commit that referenced this pull request Jul 2, 2019
@targos
2019-07-03, Version 12.6.0 (Current)
e71a0f4 
Notable changes:

* build:
  * Experimental support for building Node.js on MIPS architecture
    is back. #27992
* child_process:
  * The promisified versions of `child_process.exec` and
    `child_process.execFile` now both return a `Promise` which has the
	child instance attached to their `child` property.
	#28325
* deps:
  * Updated libuv to 1.30.1. #28449,
    #28511
    * Support for the Haiku platform has been added.
    * The maximum `UV_THREADPOOL_SIZE` has been increased from 128 to
	  1024.
    * `uv_fs_copyfile()` now works properly when the source and
	  destination files are the same.
* process:
  * A new method, `process.resourceUsage()` was added. It returns
    resource usage for the current process, such as CPU time.
	#28018
* src:
  * Fixed an issue related to stdio that could lead to a crash of the
    process in some circumstances.
	#28490
* stream:
  * Added a `writableFinished` property to writable streams. It
    indicates that all the data has been flushed to the underlying
	system. #28007
* worker:
  * Fixed an issue that prevented worker threads to listen for data on
    stdin. #28153
* meta:
  * Added Jiawen Geng (https://github.com/gengjiawen) to collaborators.
    #28322

PR-URL: #28508
targos added a commit that referenced this pull request Jul 3, 2019
@targos
2019-07-03, Version 12.6.0 (Current)
fcf8fe9 
Notable changes:

* build:
  * Experimental support for building Node.js on MIPS architecture
    is back. #27992
* child_process:
  * The promisified versions of `child_process.exec` and
    `child_process.execFile` now both return a `Promise` which has the
	child instance attached to their `child` property.
	#28325
* deps:
  * Updated libuv to 1.30.1. #28449,
    #28511
    * Support for the Haiku platform has been added.
    * The maximum `UV_THREADPOOL_SIZE` has been increased from 128 to
	  1024.
    * `uv_fs_copyfile()` now works properly when the source and
	  destination files are the same.
* process:
  * A new method, `process.resourceUsage()` was added. It returns
    resource usage for the current process, such as CPU time.
	#28018
* src:
  * Fixed an issue related to stdio that could lead to a crash of the
    process in some circumstances.
	#28490
* stream:
  * Added a `writableFinished` property to writable streams. It
    indicates that all the data has been flushed to the underlying
	system. #28007
* worker:
  * Fixed an issue that prevented worker threads to listen for data on
    stdin. #28153
* meta:
  * Added Jiawen Geng (https://github.com/gengjiawen) to collaborators.
    #28322

PR-URL: #28508
@marsam marsam mentioned this pull request Jul 3, 2019
Merged
10 tasks
This was referenced Jul 3, 2019
Closed
Closed
Closed
@bnoordhuis bnoordhuis mentioned this pull request Jul 4, 2019
Closed
bnoordhuis added a commit to bnoordhuis/io.js that referenced this pull request Jul 4, 2019
@bnoordhuis
src: block SIGTTOU before calling tcsetattr()
49acd1e 
We might be a background job that doesn't own the TTY so block SIGTTOU
before making the tcsetattr() call, otherwise that signal suspends us.

This is a better fix than PR nodejs#28490 for issue nodejs#28479.

Fixes: nodejs#28530
Fixes: nodejs#28479
Refs: nodejs#28490
pull bot pushed a commit to Mattlk13/node that referenced this pull request Jul 6, 2019
@bnoordhuis @addaleax
src: block SIGTTOU before calling tcsetattr()
b06ce0b 
We might be a background job that doesn't own the TTY so block SIGTTOU
before making the tcsetattr() call, otherwise that signal suspends us.

This is a better fix than PR nodejs#28490 for issue nodejs#28479.

Fixes: nodejs#28530
Fixes: nodejs#28479
Refs: nodejs#28490

PR-URL: nodejs#28535
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
targos pushed a commit that referenced this pull request Jul 20, 2019
@bnoordhuis @targos
src: block SIGTTOU before calling tcsetattr()
d4113f9 
We might be a background job that doesn't own the TTY so block SIGTTOU
before making the tcsetattr() call, otherwise that signal suspends us.

This is a better fix than PR #28490 for issue #28479.

Fixes: #28530
Fixes: #28479
Refs: #28490

PR-URL: #28535
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addaleax addaleax addaleax approved these changes

@targos targos targos approved these changes

@cjihrig cjihrig cjihrig approved these changes

@devsnek devsnek devsnek approved these changes

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. fast-track PRs that do not need to wait for 48 hours to land. tty Issues and PRs related to the tty subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CHECK failure in ResetStdio on >=12.5.0
7 participants
@bnoordhuis @nodejs-github-bot @targos @devsnek @addaleax @cjihrig @gireeshpunathil