Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

test: pausing and resuming stdin prevents auto-termination #1934

Closed
wants to merge 1 commit into from
Closed

test: pausing and resuming stdin prevents auto-termination #1934

wants to merge 1 commit into from

Conversation

mmalecki
Copy link

This works on node v0.4.12, but doesn't work on node v0.5.10.

This breaks apps which use process.stdin.resume() and process.stdin.pause() (it doesn't allow them to automatically terminate; such app has to call process.exit in order to exit).

@bnoordhuis
Copy link
Member

Confirmed. Issue can be reduced to new (require("tty").ReadStream)(/*fd=*/0)

@indutny
Copy link
Member

indutny commented Nov 16, 2011

Problems seem to be libuv related only:

./node -e "new (process.binding('tty_wrap').TTY)(-1, true)"

@indutny
Copy link
Member

indutny commented Nov 16, 2011

Or to be even more exact:

./node -e "new require('net').Socket(0)"

@mmalecki
Copy link
Author

@bnoordhuis, can you please pull that so that it becomes an actual problem instead of "that guy over there said it doesn't work"?

@caolan
Copy link

caolan commented Nov 20, 2011

+1, this is causing problems with my getpass implementation

@bnoordhuis
Copy link
Member

can you please pull that so that it becomes an actual problem instead of "that guy over there said it doesn't work"?

Yes, but can you rewrite the test so that it doesn't spawn a new process?

This works on `node v0.4.12`, but doesn't work on `node v0.6.2`
@mmalecki
Copy link
Author

Ah, using child process for that was in fact retarded. Done.

@bnoordhuis
Copy link
Member

Thanks, Maciej. Merged in master in 3f862cf. I'll backport it to v0.6 once a fix is out that doesn't require an API change (not likely but still).

@bnoordhuis bnoordhuis closed this Nov 21, 2011
@mmalecki
Copy link
Author

Thanks. Should I open an issue about this test failing?

@bnoordhuis
Copy link
Member

There's no real need but go ahead if you want something to track.

@caolan
Copy link

caolan commented Dec 2, 2011

@mmalecki did you open another issue for this? I'd like something to track

@mmalecki
Copy link
Author

mmalecki commented Dec 3, 2011

@caolan See #2257.

@caolan
Copy link

caolan commented Dec 3, 2011

@mmalecki thanks

This was referenced Dec 5, 2011
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants