From 1c779292315783f3c5a13cf96e66cd5898dd5a55 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 13 Feb 2018 17:41:39 +0100
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/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>
---
 doc/api/http2.md | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/doc/api/http2.md b/doc/api/http2.md
index 3e0d474151ccc2..650d1b403b20f4 100644
--- a/doc/api/http2.md
+++ b/doc/api/http2.md
@@ -1262,10 +1262,10 @@ automatically.
 const http2 = require('http2');
 const fs = require('fs');
 
-const fd = fs.openSync('/some/file', 'r');
-
 const server = http2.createServer();
 server.on('stream', (stream) => {
+  const fd = fs.openSync('/some/file', 'r');
+
   const stat = fs.fstatSync(fd);
   const headers = {
     'content-length': stat.size,
@@ -1273,8 +1273,8 @@ server.on('stream', (stream) => {
     'content-type': 'text/plain'
   };
   stream.respondWithFD(fd, headers);
+  stream.on('close', () => fs.closeSync(fd));
 });
-server.on('close', () => fs.closeSync(fd));
 ```
 
 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
 specific range subset. This can be used, for instance, to support HTTP Range
 requests.
 
+The file descriptor is not closed when the stream is closed, so it will need
+to be closed manually once it is no longer needed.
+Note that using the same file descriptor concurrently for multiple streams
+is not supported and may result in data loss. Re-using a file descriptor
+after a stream has finished is supported.
+
 When set, the `options.getTrailers()` function is called immediately after
 queuing the last chunk of payload data to be sent. The callback is passed a
 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.
 const http2 = require('http2');
 const fs = require('fs');
 
-const fd = fs.openSync('/some/file', 'r');
-
 const server = http2.createServer();
 server.on('stream', (stream) => {
+  const fd = fs.openSync('/some/file', 'r');
+
   const stat = fs.fstatSync(fd);
   const headers = {
     'content-length': stat.size,
@@ -1311,8 +1317,9 @@ server.on('stream', (stream) => {
       trailers.ABC = 'some value to send';
     }
   });
+
+  stream.on('close', () => fs.closeSync(fd));
 });
-server.on('close', () => fs.closeSync(fd));
 ```
 
 *Note*: The HTTP/1 specification forbids trailers from containing HTTP/2