Repository navigation
http2 client stops sending after stream has reached 16KiB #16578
Description
- Version: 8.8.0
- Platform: macOS High Sierra
- Subsystem: http2
When piping a readable stream to an http2 request, it sends up to 16KiB of data, then sends no more. This can be reproduced by:
The following test server will write a file when anyone connects and sends data, using readable.pipe(writable). Usage: node server.js /tmp/foo to write /tmp/foo
const http2 = require('http2');
const fs = require('fs');
const server = http2.createServer();
const filename = process.argv[2];
server.on('stream', (stream, headers) => {
console.log(headers);
stream.pipe(fs.createWriteStream(filename));
setTimeout(() => {
stream.respond({
'content-type': 'text/plain',
':status': 200
});
stream.end("1 second has elapsed");
}, 1000);
});
const port = 54321;
server.listen(port, err => {
console.log('started server on port' + port);
});The following test program reads a file and connects to an http2 server and streams the file content, also using readable.pipe(writable). Usage node client.js file-to-read.
This will succeed (small file): node client.js /etc/hosts
This will fail (large file): node client.js /usr/bin/ssh
const http2 = require('http2');
const {createReadStream} = require('fs');
const session = http2.connect("http://localhost:54321");
const req = session.request({
':path': '/',
':method': 'POST',
}, {
endStream: false,
});
const filename = process.argv[process.argv.length-1];
createReadStream(filename).pipe(req);
req.pipe(process.stdout);- Expected behaviour: Stream handling should just work with the rest of Node.
#16213 might be related
Activity
addaleax commentedon Oct 28, 2017
@nodejs/http2
apapirovski commentedon Oct 28, 2017
Looking into it, I was pretty sure we had a test for this (since I wrote it) but will check.
jasnell commentedon Oct 28, 2017
This likely has to do with the flow control mechanism. The client will pause sending if a window update is not received. I will investigate further, but it would be good to know if this works on 8.7, 8.6 and 8.5. there have been some changes in the flow control recently
jasnell commentedon Oct 28, 2017
Specifically, I'm wondering if there's a race condition that's causing a window update to not be sent...
apapirovski commentedon Oct 28, 2017
The window should be 64kb though, this seems like it sends one frame... not even the full window.
grantila commentedon Oct 28, 2017
@jasnell I'll test these versions right now
@apapirovski Stream default highWaterMark is 16KiB, isn't it?
apapirovski commentedon Oct 28, 2017
@grantila I think @jasnell is talking specifically about http2 flow control. We do have a test for something similar to this. I'll dig into what exactly is going on.
grantila commentedon Oct 28, 2017
@jasnell @apapirovski When the server runs 8.5.0, 8.6.0 and 8.7.0, the result file ends up at 64KiB. In 8.8.0 it becomes 16KiB. The client version seems irrelevant, I might be wrong.
apapirovski commentedon Oct 28, 2017
This isn't flow control, it's something about the core API and it works fine with the compatibility API. Looking into it.
(The test mentioned above runs against the compatibility API hence not noticing this regression.)
grantila commentedon Oct 28, 2017
@apapirovski right, tests for the core API would indeed be sweet. But don't misunderstand my findings, it's not necessarily a regression. The file I tried to send was +2mb, so it failed in all versions, just with different limits (64 vs 16 k)
apapirovski commentedon Oct 28, 2017
@grantila yep, it's not a regression (other than the fact that it's now more obvious because of the flow control changes on the C++ end). I've got the cause. Should have a PR shortly.
jasnell commentedon Oct 28, 2017
Where's the issue?
apapirovski commentedon Oct 29, 2017
@jasnell Sorry, took off to get dinner but here's the PR: #16580
1 remaining item
http2: fix stream reading resumption
http2: fix stream reading resumption
http2: fix stream reading resumption
http2: fix stream reading resumption
http2: fix stream reading resumption
http2: fix stream reading resumption