Skip to content
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

Do not remove idle streams immediatly. #2177

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

EvgeniiMekhanik
Copy link
Contributor

According to RFC 9113 section 5.1.1:
The first use of a new stream identifier implicitly closes all streams in the "idle" state that might
have been initiated by that peer with a lower-valued stream identifier.
But Firefox creates idle streams and uses them as a nodes in priority tree and such streams should not be deleted. So we don't remove and delete idle streams if there count is less then TFW_MAX_IDLE_STREAMS (6). Also make two fixes:

  • add/remove idle streams under ctx lock.
  • add streams to idle queue in reverse order to avoid going through all list (new stream has usually greater stream id).

@EvgeniiMekhanik EvgeniiMekhanik force-pushed the MekhanikEvgenii/do-not-delete-idle-streams branch 3 times, most recently from b20528d to f43442b Compare July 22, 2024 17:37
According to RFC 9113 section 5.1.1:
The first use of a new stream identifier implicitly
closes all streams in the "idle" state that might
have been initiated by that peer with a lower-valued
stream identifier.
But Firefox creates idle streams and uses them as a
nodes in priority tree and such streams should not be
deleted. So we don't remove and delete idle streams
if there count is less then TFW_MAX_IDLE_STREAMS (6).
Also make two fixes:
- add/remove idle streams under ctx lock.
- add streams to idle queue in reverse order to avoid
  going through all list (new stream has usually greater
  stream id).
@EvgeniiMekhanik EvgeniiMekhanik force-pushed the MekhanikEvgenii/do-not-delete-idle-streams branch from f43442b to 4b513ca Compare July 22, 2024 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant