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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
transports/quic: Adapt QuicMuxer to upstream StreamMuxer changes #6
transports/quic: Adapt QuicMuxer to upstream StreamMuxer changes #6
Changes from 1 commit
7c8a977
d4f8ec2
1a553db
e95232c
66c2319
c8066df
163c5c1
51a8471
f15a3dc
2e2c117
95713ab
f85a990
c19a211
0ec3bbc
74f01e4
ce963df
56c492c
09c6908
eaf3f3a
7019d49
2b9e212
028dece
07c0dba
57840a3
579b1be
e2b83b7
3da8b42
1012579
a4110a2
0a01c81
3ce0ef9
3060d12
63c6edc
06aaea6
cef5056
0e5a25d
878c49f
6a9fa3d
8dc0188
67266c6
d2c5053
a2738fd
475289c
8931860
1aeaba3
95fc6da
3d3666e
217dd2c
0d7c8a5
67b52aa
66974fc
4253080
c88efe8
0a82be4
d610e4b
d92cab8
ca07ce4
a3dec47
247b553
6855ab9
e01f77b
f16561c
36a2773
89f898c
cee199a
f04df29
b8c3b28
a40180c
8644c65
2eca38c
d2eddf4
83c6795
c650dc1
69caf98
fe3e09b
b6924db
689460f
457fb51
41d39fb
66c2755
72bade1
5906140
2c739e9
c81b06a
2025de3
4c617a0
4e027b1
bdba780
40cb4f3
f8d1430
4c3229b
e393fe5
d28db18
42db0ed
d46b72e
ec3c74a
b7103aa
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
error
seems a bit much here. I wouldn't won't be woken at 3am because my production app is reporting _error_s and then seeing it is this one which is practically harmless :)Why is this unexpected in the first place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ConnectionEvent::Connected
should only be returned a single time, which is when we finished all the crypto and established a connection.In
quic::upgrade::Update
we poll a pending new connection until it returnsConnectionEvent::Connected
. Only then we create theQuicMuxer
for this connection. Hence withinQuicMuxer
the event should not happen again.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it really should not happen, then perhaps put a debug assert here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's assume this returns
Poll::Pending
at some point. Then we will break out of this loop and register a waker further down. Once we are woken,poll_close
gets called again and we go straight intoinner.poll_connection
which will likely yield theConnectionEvent::ConnectionLost
. I don't think we will be observing it here then, right?I think we may not want to call
poll_connection
at the top here or maybe return an error frompoll_connection
in case it has been closed?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we upholding the poll-contract in case the list of substreams is not empty?