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

net.listen does not emit 'listening' event when in --eval mode #1600

Closed
rmg opened this issue May 3, 2015 · 11 comments
Closed

net.listen does not emit 'listening' event when in --eval mode #1600

rmg opened this issue May 3, 2015 · 11 comments
Assignees
Labels
confirmed-bug Issues with confirmed bugs. net Issues and PRs related to the net subsystem.

Comments

@rmg
Copy link
Contributor

rmg commented May 3, 2015

Failing unit test submitted as PR #1581

I found this when trying to confirm the format of the address object. The following hangs on most releases of node higher than 0.10.38:

node -e "require('net').createServer().listen(0, function() { console.log(this.address()); this.close(); });"

It seems that Server does not emit a listen event when no address is specified and is called in eval mode.

I don't fully understand how the regression was introduced, but git bisect seemed pretty confident that it was in 5b636fe.

/to @trevnorris for the indicated commit
/cc @piscisaureus @sam-github @brendanashworth

@mscdex mscdex added the net Issues and PRs related to the net subsystem. label May 3, 2015
@brendanashworth brendanashworth added the confirmed-bug Issues with confirmed bugs. label May 4, 2015
@trevnorris trevnorris self-assigned this May 4, 2015
@trevnorris
Copy link
Contributor

wtf. That commit was 2 years ago?

Assigning this to myself.

@rmg
Copy link
Contributor Author

rmg commented May 5, 2015

@trevnorris agreed, seriously wtf. Looking forward to the fix, 'cause I have no idea..

@trevnorris
Copy link
Contributor

Just to clarify, it doesn't matter what port number is passed.

@trevnorris
Copy link
Contributor

The call to emitListeningNT isn't getting called in lib/net.js. Unfortunately none of the code linked in the bisect event exists anymore, so the issue must have just propagated strangely.

UPDATE: _tickCallback() is never reached, so nothing in the nextTickQueue is run. Figuring out why.

UPDATE: Seems that evalScript() from src/node.js simply runs the script in vm.runInThisContext(). Which doesn't cause the nextTickQueue to be processed for some reason.

UPDATE: Reason the nextTickQueue isn't processed is because the call to .listen() is actually synchronous. Check the call stack:

    at Server._listen2 (net.js:1210:22)
    at listen (net.js:1228:10)
    at Server.listen (net.js:1318:5)
    at [eval]:1:31
    at Object.exports.runInThisContext (vm.js:54:17)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:431:26)
    at evalScript (node.js:567:25)
    at startup (node.js:95:9)
    at node.js:959:3

I'm open to ideas of the best way to get the call stack to flip over.

@trevnorris
Copy link
Contributor

@rmg So I've determined, mostly, why it's happening, but I'm not sure of the best fix. Thoughts?

@brendanashworth
Copy link
Contributor

Cross linking to nodejs/node-v0.x-archive#14168.

@bnoordhuis
Copy link
Member

There's an easy "fix" (maybe "hack" is more appropriate): defer evaluation for a tick.

diff --git a/src/node.js b/src/node.js
index cb50341..3a7e3c9 100644
--- a/src/node.js
+++ b/src/node.js
@@ -558,8 +558,10 @@
              'return require("vm").runInThisContext(' +
              JSON.stringify(body) + ', { filename: ' +
              JSON.stringify(name) + ' });\n';
-    var result = module._compile(script, name + '-wrapper');
-    if (process._print_eval) console.log(result);
+    process.nextTick(function() {
+      var result = module._compile(script, name + '-wrapper');
+      if (process._print_eval) console.log(result);
+    });
   }

   function createWritableStdioStream(fd) {

@trevnorris
Copy link
Contributor

@bnoordhuis Nice. Want to handle the PR for that, or want me to?

@bnoordhuis
Copy link
Member

I'll file a PR, I don't think I've had a commit in for a while now. :-)

bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue May 25, 2015
Defer evaluation of the script for a tick.  This is a workaround for
events not firing when evaluating scripts on the command line with -e.

Fixes: nodejs#1600
@brendanashworth
Copy link
Contributor

Fix: #1793

bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue May 26, 2015
Defer evaluation of the script for a tick.  This is a workaround for
events not firing when evaluating scripts on the command line with -e.

Fixes: nodejs#1600
bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue May 26, 2015
Defer evaluation of the script for a tick.  This is a workaround for
events not firing when evaluating scripts on the command line with -e.

Fixes: nodejs#1600
PR-URL: nodejs#1793
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@bnoordhuis
Copy link
Member

Fixed by 93a44d5.

andrewdeandrade pushed a commit to andrewdeandrade/node that referenced this issue Jun 3, 2015
Defer evaluation of the script for a tick.  This is a workaround for
events not firing when evaluating scripts on the command line with -e.

Fixes: nodejs/node#1600
PR-URL: nodejs/node#1793
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. net Issues and PRs related to the net subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants