Commit eb9d0ba
doc: warn against concurrent http2stream.respondWithFD
Upcoming changes to move away from synchronous I/O on the main
thread will imply that using the same file descriptor to
respond on multiple HTTP/2 streams at the same time is invalid,
because at least on Windows `uv_fs_read()` is race-y.
Therefore, warn against such usage.
PR-URL: #18762
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>1 parent c569f70 commit eb9d0ba
1 file changed
Lines changed: 13 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1262,19 +1262,19 @@ automatically. | |||
| 1262 | 1262 | const http2 = require('http2'); | |
| 1263 | 1263 | const fs = require('fs'); | |
| 1264 | 1264 | ||
| 1265 | - const fd = fs.openSync('/some/file', 'r'); | ||
| 1266 | - | ||
| 1267 | 1265 | const server = http2.createServer(); | |
| 1268 | 1266 | server.on('stream', (stream) => { | |
| 1267 | + const fd = fs.openSync('/some/file', 'r'); | ||
| 1268 | + | ||
| 1269 | 1269 | const stat = fs.fstatSync(fd); | |
| 1270 | 1270 | const headers = { | |
| 1271 | 1271 | 'content-length': stat.size, | |
| 1272 | 1272 | 'last-modified': stat.mtime.toUTCString(), | |
| 1273 | 1273 | 'content-type': 'text/plain' | |
| 1274 | 1274 | }; | |
| 1275 | 1275 | stream.respondWithFD(fd, headers); | |
| 1276 | + stream.on('close', () => fs.closeSync(fd)); | ||
| 1276 | 1277 | }); | |
| 1277 | - server.on('close', () => fs.closeSync(fd)); | ||
| 1278 | 1278 | ``` | |
| 1279 | 1279 | ||
| 1280 | 1280 | The optional `options.statCheck` function may be specified to give user code | |
@@ -1287,6 +1287,12 @@ The `offset` and `length` options may be used to limit the response to a | |||
| 1287 | 1287 | specific range subset. This can be used, for instance, to support HTTP Range | |
| 1288 | 1288 | requests. | |
| 1289 | 1289 | ||
| 1290 | + The file descriptor is not closed when the stream is closed, so it will need | ||
| 1291 | + to be closed manually once it is no longer needed. | ||
| 1292 | + Note that using the same file descriptor concurrently for multiple streams | ||
| 1293 | + is not supported and may result in data loss. Re-using a file descriptor | ||
| 1294 | + after a stream has finished is supported. | ||
| 1295 | + | ||
| 1290 | 1296 | When set, the `options.getTrailers()` function is called immediately after | |
| 1291 | 1297 | queuing the last chunk of payload data to be sent. The callback is passed a | |
| 1292 | 1298 | single object (with a `null` prototype) that the listener may use to specify | |
@@ -1296,10 +1302,10 @@ the trailing header fields to send to the peer. | |||
| 1296 | 1302 | const http2 = require('http2'); | |
| 1297 | 1303 | const fs = require('fs'); | |
| 1298 | 1304 | ||
| 1299 | - const fd = fs.openSync('/some/file', 'r'); | ||
| 1300 | - | ||
| 1301 | 1305 | const server = http2.createServer(); | |
| 1302 | 1306 | server.on('stream', (stream) => { | |
| 1307 | + const fd = fs.openSync('/some/file', 'r'); | ||
| 1308 | + | ||
| 1303 | 1309 | const stat = fs.fstatSync(fd); | |
| 1304 | 1310 | const headers = { | |
| 1305 | 1311 | 'content-length': stat.size, | |
@@ -1311,8 +1317,9 @@ server.on('stream', (stream) => { | |||
| 1311 | 1317 | trailers.ABC = 'some value to send'; | |
| 1312 | 1318 | } | |
| 1313 | 1319 | }); | |
| 1320 | + | ||
| 1321 | + stream.on('close', () => fs.closeSync(fd)); | ||
| 1314 | 1322 | }); | |
| 1315 | - server.on('close', () => fs.closeSync(fd)); | ||
| 1316 | 1323 | ``` | |
| 1317 | 1324 | ||
| 1318 | 1325 | *Note*: The HTTP/1 specification forbids trailers from containing HTTP/2 | |
0 commit comments