search

spdy-http2 / node-spdy

SPDY server on Node.js
2.81k stars 196 forks source link

server_push Crash with fs.createReadStream.pipe() #318

Open yayo opened 7 years ago

yayo commented 7 years ago

assert.js:81 throw new assert.AssertionError({ ^ AssertionError: false == true at PriorityNode.removeChild (/tmp/cmbc/node_modules/spdy-transport/lib/spdy-transport/priority.js:72:3) at PriorityNode.remove (/tmp/cmbc/node_modules/spdy-transport/lib/spdy-transport/priority.js:61:15) at PriorityTree.add (/tmp/cmbc/node_modules/spdy-transport/lib/spdy-transport/priority.js:157:23) at PriorityTree.addDefault (/tmp/cmbc/node_modules/spdy-transport/lib/spdy-transport/priority.js:174:15) at Stream._initPriority (/tmp/cmbc/node_modules/spdy-transport/lib/spdy-transport/stream.js:97:27) at new Stream (/tmp/cmbc/node_modules/spdy-transport/lib/spdy-transport/stream.js:75:8) at Connection._createStream (/tmp/cmbc/node_modules/spdy-transport/lib/spdy-transport/connection.js:391:16) at Connection.pushPromise (/tmp/cmbc/node_modules/spdy-transport/lib/spdy-transport/connection.js:772:21) at Stream.pushPromise (/tmp/cmbc/node_modules/spdy-transport/lib/spdy-transport/stream.js:673:30) at ServerResponse.push (/tmp/cmbc/node_modules/spdy/lib/spdy/response.js:92:17)

function(req,res) { for(i=1;i<=51;i++) {var f="/"+i; var s=res.push(f,{request:{accept:'/'},response:{'content-type':'application/javascript'}}); s.on('error',function(){}); fs.createReadStream(process.argv[1],{autoClose:true}).pipe(s); / CRASHED / //s.end(fs.readFileSync(process.argv[1])); / OK / } res.end(); }

refresh (F5) your browser quickly twice(or more), and it will crash ! may cause by exceed "transport.protocol.base.constants.MAX_PRIORITY_STREAMS"

WVmf commented 7 years ago

This is probably still the same issue as #285 and #244

daviddias commented 7 years ago

Seems it is not happy when several push streams are being created and the connection drops half way through.

@yayo could you try to convert this to a test?

matthewp commented 7 years ago

@diasdavid I've recreated the @yayo example as a gist here: https://gist.github.com/matthewp/d5a26ba620fc921754f3ba4892a9b230

I tried to make it into a test but I was not successful. I think I don't understand the lower-level apis well enough yet. Is there a similar test that I could use as an example?

Either way, what's the best way to debug this? Currently a bit of a blocker for me.

matthewp commented 7 years ago

An update on what is going on here. The issue is in spdy-transport.

  1. spdy-transport has a maxCount option, and when that goes over it removes the first node in the tree here.
  2. When there is a priority frame the first node also gets removed here.

Since the node was removed in (1) it fails in (2). I'm not sure what the proper solution is here @diasdavid @indutny. Should _handlePriority check if the node is still in the tree before calling remove() or is there some other approach here?

matthewp commented 7 years ago

The root problem is that _removeNode removes the node from the map but not the list. Fixing that ensures that it is removed from the tree. This also fixes a memory leak.

matthewp commented 7 years ago

PR: https://github.com/spdy-http2/spdy-transport/pull/48

veaba commented 5 years ago

Is there a good way?