http2: backport changes to v9.x-staging by jasnell · Pull Request #18050 · nodejs/node
added
lib / src
v9.x labels
jasnell
changed the title
Backport http2 v9.x
http2: backport changes to v9.x-staging
Adds the possibility to keep a strong persistent reference to a JS object while a `SetImmediate()` call is in effect. PR-URL: nodejs#17183 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Calling into JS land from GC is not allowed, so delay the resolution of pending pings when a session is destroyed. PR-URL: nodejs#17183 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Introduce an `Http2Scope` class that, when it goes out of scope, checks whether a write to the network is desired by nghttp2. If that is the case, schedule a write using `SetImmediate()` rather than a custom per-session libuv handle. PR-URL: nodejs#17183 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
PR-URL: nodejs#17406 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
PR-URL: nodejs#17406 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> This is a significant cleanup and refactoring of the cleanup/close/destroy logic for Http2Stream and Http2Session. There are significant changes here in the timing and ordering of cleanup logic, JS apis. and various related necessary edits.
`nghttp2_stream_write_t` was not a necessary redirection layer and came with the cost of one additional allocation per stream write. Also, having both `nghttp2_stream_write` and `nghttp2_stream_write_t` as identifiers did not help with readability. PR-URL: nodejs#17718 Reviewed-By: James M Snell <jasnell@gmail.com>
- Only finish outgoing `WriteWrap`s once data has actually been passed to the underlying socket. - This makes HTTP2 streams respect backpressure - Use `DoTryWrite` as a shortcut for sending out as much of the data synchronously without blocking as possible - Use `NGHTTP2_DATA_FLAG_NO_COPY` to avoid copying DATA frame contents into nghttp2’s buffers before sending them out. PR-URL: nodejs#17718 Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#17863 Fixes: nodejs#17840 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Keep a local handle as a reference to the JS `Http2Session` object so that it will not be garbage collected when inside an `Http2Scope`, because the presence of the latter usually indicates that further actions on the session object are expected. Strictly speaking, storing the `session_handle_` as a property on the scope object is not necessary, but this is not very costly and makes the code more obviously correct. Fixes: nodejs#17840 PR-URL: nodejs#17863 Fixes: nodejs#17840 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#17620 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#17939 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Adds the possibility to keep a strong persistent reference to a JS object while a `SetImmediate()` call is in effect. Backport-PR-URL: #18050 PR-URL: #17183 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins pushed a commit that referenced this pull request
Calling into JS land from GC is not allowed, so delay the resolution of pending pings when a session is destroyed. Backport-PR-URL: #18050 PR-URL: #17183 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins pushed a commit that referenced this pull request
Introduce an `Http2Scope` class that, when it goes out of scope, checks whether a write to the network is desired by nghttp2. If that is the case, schedule a write using `SetImmediate()` rather than a custom per-session libuv handle. Backport-PR-URL: #18050 PR-URL: #17183 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins pushed a commit that referenced this pull request
MylesBorins pushed a commit that referenced this pull request
Backport-PR-URL: #18050 PR-URL: #17406 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> This is a significant cleanup and refactoring of the cleanup/close/destroy logic for Http2Stream and Http2Session. There are significant changes here in the timing and ordering of cleanup logic, JS apis. and various related necessary edits.
MylesBorins pushed a commit that referenced this pull request
`nghttp2_stream_write_t` was not a necessary redirection layer and came with the cost of one additional allocation per stream write. Also, having both `nghttp2_stream_write` and `nghttp2_stream_write_t` as identifiers did not help with readability. Backport-PR-URL: #18050 PR-URL: #17718 Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request
- Only finish outgoing `WriteWrap`s once data has actually been passed to the underlying socket. - This makes HTTP2 streams respect backpressure - Use `DoTryWrite` as a shortcut for sending out as much of the data synchronously without blocking as possible - Use `NGHTTP2_DATA_FLAG_NO_COPY` to avoid copying DATA frame contents into nghttp2’s buffers before sending them out. Backport-PR-URL: #18050 PR-URL: #17718 Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request
MylesBorins pushed a commit that referenced this pull request
MylesBorins pushed a commit that referenced this pull request
Keep a local handle as a reference to the JS `Http2Session` object so that it will not be garbage collected when inside an `Http2Scope`, because the presence of the latter usually indicates that further actions on the session object are expected. Strictly speaking, storing the `session_handle_` as a property on the scope object is not necessary, but this is not very costly and makes the code more obviously correct. Fixes: #17840 Backport-PR-URL: #18050 PR-URL: #17863 Fixes: #17840 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request
MylesBorins pushed a commit that referenced this pull request
MylesBorins pushed a commit that referenced this pull request
MylesBorins pushed a commit that referenced this pull request
Strictly limit the number of concurrent streams based on the current setting of the MAX_CONCURRENT_STREAMS setting Backport-PR-URL: #18050 PR-URL: #16766 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Sebastiaan Deckers <sebdeckers83@gmail.com>
MylesBorins pushed a commit that referenced this pull request
MylesBorins pushed a commit that referenced this pull request
MylesBorins pushed a commit that referenced this pull request
Add new properties to `Http2Session` to identify alpnProtocol, and indicator about whether the session is TLS or not, and initial support for origin set (preparinng for `ORIGIN` frame support and the client-side `Pool` implementation. The `originSet` is the set of origins for which an `Http2Session` may be considered authoritative. Per the `ORIGIN` frame spec, the originSet is only valid on TLS connections, so this is only exposed when using a `TLSSocket`. Backport-PR-URL: #18050 PR-URL: #17935 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Sebastiaan Deckers <sebdeckers83@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
MylesBorins pushed a commit that referenced this pull request
MylesBorins pushed a commit that referenced this pull request
MylesBorins pushed a commit that referenced this pull request
MylesBorins pushed a commit that referenced this pull request
The maxSessionMemory is a cap for the amount of memory an Http2Session is permitted to consume. If exceeded, new `Http2Stream` sessions will be rejected with an `ENHANCE_YOUR_CALM` error and existing `Http2Stream` instances that are still receiving headers will be terminated with an `ENHANCE_YOUR_CALM` error. Backport-PR-URL: #18050 PR-URL: #17967 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
MylesBorins pushed a commit that referenced this pull request
MylesBorins pushed a commit that referenced this pull request
Backport-PR-URL: #18050 PR-URL: #17972 Reviewed-By: Weijia Wang <starkwang@126.com> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
MylesBorins pushed a commit that referenced this pull request
* verify protections against ping and settings flooding * Strictly handle and verify handling of unsolicited ping and settings frame acks. Backport-PR-URL: #18050 PR-URL: #17969 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Backport-PR-URL: #18050 PR-URL: #17911 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Weijia Wang <starkwang@126.com>
MylesBorins pushed a commit that referenced this pull request
Scheduling a PerformanceGCCallback should not keep the loop alive but due to the recent switch to using the native SetImmediate method, it does. Go back to using uv_async_t and add a regression test. Backport-PR-URL: #18050 PR-URL: #18051 Fixes: #18047 Refs: #18020 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request
Introduce an `Http2Scope` class that, when it goes out of scope, checks whether a write to the network is desired by nghttp2. If that is the case, schedule a write using `SetImmediate()` rather than a custom per-session libuv handle. Backport-PR-URL: nodejs#18050 PR-URL: nodejs#17183 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>