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

Pasting large amount of text into REPL causes assertion failure #9690

Closed
seishun opened this issue Nov 18, 2016 · 6 comments
Closed

Pasting large amount of text into REPL causes assertion failure #9690

seishun opened this issue Nov 18, 2016 · 6 comments
Labels
libuv Issues and PRs related to the libuv dependency or the uv binding. repl Issues and PRs related to the REPL subsystem. windows Issues and PRs related to the Windows platform.

Comments

@seishun
Copy link
Contributor

seishun commented Nov 18, 2016

  • Version: master
  • Platform: Windows 10 64-bit
  • Subsystem: libuv
  1. Build a debug build of node using vcbuild nosign debug
  2. Run Debug\node.exe
  3. Paste a large amount of text into REPL.

Output:

Assertion failed: req != current, file c:\users\nikolai\downloads\node_clean\deps\uv\src\win\req-inl.h, line 102

Example text:

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
@silverwind silverwind added the repl Issues and PRs related to the REPL subsystem. label Nov 18, 2016
@doodadjs
Copy link

I also have this issue with "console.log" with Node.js version 7.1.0 on Windows 10 64 bits. Running the following command will make Node crash. That was initially a stack trace of 9603 chars.

console.log("0".repeat(9603));

@mscdex mscdex added the windows Issues and PRs related to the Windows platform. label Nov 19, 2016
@Fishrock123
Copy link
Contributor

I think this is a dupe of #9542 fixed by 1fca11e

@seishun
Copy link
Contributor Author

seishun commented Nov 19, 2016

@Fishrock123 nope, I'm using master which contains this commit. Unless you were responding to the comment above.

@doodadjs I don't have such issue in master, so it's probably already fixed.

@seishun
Copy link
Contributor Author

seishun commented Nov 19, 2016

I think I've found the issue.

  1. Node.js calls _setRawMode twice in the same tick:

    node/lib/repl.js

    Lines 296 to 314 in 2e28875

    previouslyInRawMode = self._setRawMode(false);
    }
    try {
    try {
    const scriptOptions = {
    displayErrors: true,
    breakOnSigint: self.breakEvalOnSigint
    };
    if (self.useGlobal) {
    result = script.runInThisContext(scriptOptions);
    } else {
    result = script.runInContext(context, scriptOptions);
    }
    } finally {
    if (self.breakEvalOnSigint) {
    // Reset terminal mode to its previous value.
    self._setRawMode(previouslyInRawMode);
    Both times libuv stops and restarts reading tty.
  2. Both times, since there is pending data due to overfilled buffer, libuv calls uv_insert_pending_req with &handle->read_req:

    node/deps/uv/src/win/tty.c

    Lines 989 to 993 in 1fca11e

    if (handle->tty.rd.last_key_len > 0) {
    SET_REQ_SUCCESS(&handle->read_req);
    uv_insert_pending_req(handle->loop, (uv_req_t*) &handle->read_req);
    return 0;
    }

    The second time it causes the assertion failure since it's already inserted.

I don't understand the logic behind the uv_insert_pending_req call. Pinging @piscisaureus @saghul

@addaleax
Copy link
Member

@seishun thanks for investigating… I’ve marked #8645 as dont-land for now

@seishun
Copy link
Contributor Author

seishun commented Dec 6, 2016

Reopening until libuv is upgraded.

@seishun seishun reopened this Dec 6, 2016
cjihrig added a commit to cjihrig/node that referenced this issue Jan 12, 2017
Refs: nodejs#9439
Fixes: nodejs#9464
Fixes: nodejs#9690
PR-URL: nodejs#10717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 18, 2017
Refs: nodejs#9439
Fixes: nodejs#9464
Fixes: nodejs#9690
PR-URL: nodejs#10717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 23, 2017
Refs: nodejs#9439
Fixes: nodejs#9464
Fixes: nodejs#9690
PR-URL: nodejs#10717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 25, 2017
Refs: nodejs#9439
Fixes: nodejs#9464
Fixes: nodejs#9690
PR-URL: nodejs#10717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 27, 2017
Refs: nodejs#9439
Fixes: nodejs#9464
Fixes: nodejs#9690
PR-URL: nodejs#10717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
MylesBorins pushed a commit that referenced this issue May 16, 2017
Refs: #9439
Fixes: #9464
Fixes: #9690
PR-URL: #10717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
MylesBorins pushed a commit that referenced this issue May 18, 2017
Refs: #9439
Fixes: #9464
Fixes: #9690
PR-URL: #10717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
andrew749 pushed a commit to michielbaird/node that referenced this issue Jul 19, 2017
Refs: nodejs/node#9439
Fixes: nodejs/node#9464
Fixes: nodejs/node#9690
PR-URL: nodejs/node#10717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libuv Issues and PRs related to the libuv dependency or the uv binding. repl Issues and PRs related to the REPL subsystem. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

6 participants