http2: backport changes to v9.x-staging#18050
Closed
jasnell wants to merge 27 commits intonodejs:v9.x-stagingfrom
Closed
http2: backport changes to v9.x-staging#18050jasnell wants to merge 27 commits intonodejs:v9.x-stagingfrom
jasnell wants to merge 27 commits intonodejs:v9.x-stagingfrom
Conversation
Member
|
v9.x-staging fails because #17704 is not in it. Is it possible to include it in this PR? |
Member
Author
|
Not sure. That's a semver-major pr so would need a semver-major minor backport that omitted the breaking bits. That would best be done in a separate PR. |
Member
|
Could this commit be further backported to 8 in case? |
Member
Author
|
That's going to require a bit more work. Planning to do it later this week |
bd744b1 to
f55a5e8
Compare
bf3c880 to
512fa41
Compare
Contributor
|
I've rebased against the new v9.x-staging to remove a commit that was breaking the build, it looks like another commit may be breaking stuff so i'll likely rebase one more time 😄 |
Member
|
@MylesBorins you are looking for #17800 which depends on #17704 |
Contributor
|
@targos you are a saint! I was just digging in |
f55a5e8 to
30273d4
Compare
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 <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
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 <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
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 <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
PR-URL: nodejs#17406 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
PR-URL: nodejs#17406 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> 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 <[email protected]>
- 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 <[email protected]>
PR-URL: nodejs#17763 Refs: nodejs#17746 Reviewed-By: Matteo Collina <[email protected]>
PR-URL: nodejs#17863 Fixes: nodejs#17840 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs#17620 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: nodejs#17939 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: James M Snell <[email protected]>
Member
|
LGTM |
|
T |
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
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 <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
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 <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
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 <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
PR-URL: #17406 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Backport-PR-URL: #18050
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
Backport-PR-URL: #18050 PR-URL: #17406 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> 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
Jan 10, 2018
`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 <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
- 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 <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
Backport-PR-URL: #18050 PR-URL: #17763 Refs: #17746 Reviewed-By: Matteo Collina <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
Backport-PR-URL: #18050 PR-URL: #17863 Fixes: #17840 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
Backport-PR-URL: #18050 PR-URL: #17620 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
Backport-PR-URL: #18050 PR-URL: #17939 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
Collect and report basic timing information about `Http2Session` and `Http2Stream` instances. Backport-PR-URL: #18050 PR-URL: #17906 Refs: #17746 Reviewed-By: Matteo Collina <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
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 <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Sebastiaan Deckers <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
Add support for sending and receiving ALTSVC frames. Backport-PR-URL: #18050 PR-URL: #17917 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
Backport-PR-URL: #18050 PR-URL: #17935 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Sebastiaan Deckers <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
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 <[email protected]> Reviewed-By: Sebastiaan Deckers <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
Add a padding strategy option that makes a best attempt to ensure that total frame length for DATA and HEADERS frames are aligned on multiples of 8-bytes. Backport-PR-URL: #18050 PR-URL: #17938 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
Backport-PR-URL: #18050 PR-URL: #17942 Reviewed-By: Matteo Collina <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
Backport-PR-URL: #18050 PR-URL: #17942 Reviewed-By: Matteo Collina <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
Backport-PR-URL: #18050 PR-URL: #17954 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
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 <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
Backport-PR-URL: #18050 PR-URL: #17968 Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
Backport-PR-URL: #18050 PR-URL: #17972 Reviewed-By: Weijia Wang <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
* 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 <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
Backport-PR-URL: #18050 PR-URL: #17911 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Weijia Wang <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 10, 2018
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 <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins
pushed a commit
to MylesBorins/node
that referenced
this pull request
Apr 12, 2018
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 <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of multiple http2 related commits to v9.x-staging.
Currently blocked by #18049 and whatever else is currently causing v9.x-staging to fail. Will be able to finish once v9.x-staging is functioning again.
Note: have not fully tested this yet because of the v9.x-staging breakage.
Ping @nodejs/http2 @mcollina
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
http2