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

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

Open
yayo opened this issue May 21, 2017 · 7 comments
Open

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

yayo opened this issue May 21, 2017 · 7 comments

Comments

@yayo
Copy link

yayo commented May 21, 2017

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
Copy link

WVmf commented May 29, 2017

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

@daviddias
Copy link
Member

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
Copy link

matthewp commented Jun 23, 2017

@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
Copy link

matthewp commented Jun 26, 2017

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
Copy link

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
Copy link

PR: spdy-http2/spdy-transport#48

matthewp added a commit to donejs/spdy-transport that referenced this issue Jun 26, 2017
When a node is removed it gets removed from the tree's `map` but not the
tree's `list`, so this memory is leaked.

This also fixes the assertion errors seen in spdy-http2#47 and
spdy-http2/node-spdy#318 and elsewhere.
veaba added a commit to veaba/express-nuxt that referenced this issue Sep 12, 2018
1、发现 使用backpack debug dev 导致程序卡死,然后刷新页面。相关问题 spdy-http2/node-spdy#318
2、删了debug 就好。等待后续测试
@veaba
Copy link

veaba commented Sep 12, 2018

Is there a good way?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants