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

HTTP: Cannot read property 'Symbol(asyncId)' of null #14368

Closed
tutman96 opened this issue Jul 19, 2017 · 9 comments
Closed

HTTP: Cannot read property 'Symbol(asyncId)' of null #14368

tutman96 opened this issue Jul 19, 2017 · 9 comments
Labels
async_hooks Issues and PRs related to the async hooks subsystem. confirmed-bug Issues with confirmed bugs. http Issues or PRs related to the http subsystem.

Comments

@tutman96
Copy link

  • Version: 8.0.0 - latest
  • Platform: Darwin 16.6.0 Darwin Kernel Version 16.6.0: Fri Apr 14 16:21:16 PDT 2017; root:xnu-3789.60.24~6/RELEASE_X86_64 x86_64
  • Subsystem: http

It appears like the addition of async_hooks API into the http module (commit 4a7233c) causes an edge case to occur with 'write after end'. Here is a stack trace:

TypeError: Cannot read property 'Symbol(asyncId)' of null
    at write_ (_http_outgoing.js:636:24)
    at ServerResponse.write (_http_outgoing.js:630:10)
    at Immediate.setImmediate [as _onImmediate] (/test.js:10:7)
    at runCallback (timers.js:800:20)
    at tryOnImmediate (timers.js:762:5)
    at processImmediate [as _immediateCallback] (timers.js:733:5)

This is an unhandled exception, thrown all the way up to OutgoingMessage.prototype.write, causing crashes when used with streams in some cases. It appears like msg.socket[async_id_symbol] is causing the error due to msg.socket being null. I have slimmed down the code into as bare as I could. Here is my code:

var http = require('http');

// This code crashes the process
http.createServer((req, res) => {
    res.on("error", (err) => console.error("res had error:", err));
    
    res.write('hello');
    res.end();
    setImmediate(() => {
        res.write('world')
    })
}).listen(9000);

// This code works as expected
http.createServer((req, res) => {
    res.on("error", (err) => console.error("res had error:", err));
    
    res.write('hello');
    res.end();
    res.write('world')
}).listen(9001);

The second block works as intended (emits the error event). It appears like this bug occurs in all versions after v8.0.0.

@mscdex mscdex added http Issues or PRs related to the http subsystem. async_hooks Issues and PRs related to the async hooks subsystem. labels Jul 19, 2017
@mscdex
Copy link
Contributor

mscdex commented Jul 19, 2017

/cc @nodejs/async_hooks

@AndreasMadsen
Copy link
Member

Thanks for the issue report. This appears to be already have been fixed (at least when I try). Can you try the nightly build and confirm?

@mscdex mscdex added the v8.x label Jul 19, 2017
@tutman96
Copy link
Author

tutman96 commented Jul 19, 2017

Sure, just tried it. Here is the log output (called http://localhost:9001 and then http://localhost:9000):

res had error: Error: write after end
    at write_ (_http_outgoing.js:644:15)
    at ServerResponse.write (_http_outgoing.js:639:10)
    at Server.http.createServer (/test.js:18:6)
    at emitTwo (events.js:125:13)
    at Server.emit (events.js:213:7)
    at parserOnIncoming (_http_server.js:603:12)
    at HTTPParser.parserOnHeadersComplete (_http_common.js:116:23)
_http_outgoing.js:645
    nextTick(msg.socket[async_id_symbol],
                        ^

TypeError: Cannot read property 'Symbol(asyncId)' of null
    at write_ (_http_outgoing.js:645:25)
    at ServerResponse.write (_http_outgoing.js:639:10)
    at Immediate.setImmediate [as _onImmediate] (/test.js:9:7)
    at runCallback (timers.js:781:20)
    at tryOnImmediate (timers.js:743:5)
    at processImmediate [as _immediateCallback] (timers.js:714:5)

(node -v = v9.0.0-nightly20170718f406a7ebae)

@AndreasMadsen
Copy link
Member

Ah, okay. I can reproduce this, thanks :)

@AndreasMadsen
Copy link
Member

AndreasMadsen commented Jul 19, 2017

@refack I'm not going to have time to debug/fix this, could you look at it.

I think msg.socket === null ? null : msg.socket[async_id_symbol] is a resonable solution. That being said, having msg.socket[async_id_symbol] after the trigger is destroyed is kinda odd to me, so there might be more to it.

@AndreasMadsen AndreasMadsen added the confirmed-bug Issues with confirmed bugs. label Jul 19, 2017
@refack
Copy link
Contributor

refack commented Jul 19, 2017

@refack I'm not going to have time to debug/fix this, could you look at it.

Will do

@mcollina
Copy link
Member

@refack do you have a PR for this?

@mcollina
Copy link
Member

I've included a fix for this in #14387.

@refack
Copy link
Contributor

refack commented Jul 20, 2017

@refack do you have a PR for this?

I was still fighting with reproducing, so 👍 on #14387

mcollina added a commit to mcollina/node that referenced this issue Jul 20, 2017
Fishrock123 pushed a commit that referenced this issue Jul 20, 2017
Fixes: #14368

PR-URL: #14387
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
addaleax pushed a commit that referenced this issue Jul 22, 2017
Fixes: #14368

PR-URL: #14387
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem. confirmed-bug Issues with confirmed bugs. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants