Skip to content

Fix deleteting previosly closed streams.#2351

Merged
krizhanovsky merged 2 commits into
masterfrom
MekhanikEvgenii/fix-1411-2
Mar 11, 2025
Merged

Fix deleteting previosly closed streams.#2351
krizhanovsky merged 2 commits into
masterfrom
MekhanikEvgenii/fix-1411-2

Conversation

@EvgeniiMekhanik

Copy link
Copy Markdown
Contributor

Previously we delete previously closed streams in
case when count of closed streams is greater then
TFW_MAX_CLOSED_STREAMS. But there is a case when
count of streams is equal to max concurrent streams when we should close all closed streams to be able to create new streams.

Previously we delete previously closed streams in
case when count of closed streams is greater then
TFW_MAX_CLOSED_STREAMS. But there is a case when
count of streams is equal to max concurrent streams
when we should close all closed streams to be able
to create new streams.
We should immediately return in case of error
not use '|=' to collect result.
@EvgeniiMekhanik EvgeniiMekhanik added this to the 0.8 - Beta milestone Mar 8, 2025
@krizhanovsky krizhanovsky removed their request for review March 8, 2025 05:42
Comment thread fw/http2.c
if (closed_streams->num <= TFW_MAX_CLOSED_STREAMS) {
if (closed_streams->num <= TFW_MAX_CLOSED_STREAMS
&& (ctx->streams_num < max_streams
|| !closed_streams->num)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with current solution, however I would like to ask, maybe we need account ctx->streams_num excluding closed_streams->num?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No! Streams moved to closed_streams queue after sending all data to the client or when RST stream is received. If we don't take them into account we never delete this streams from memory

@krizhanovsky krizhanovsky merged commit 11853e1 into master Mar 11, 2025
@krizhanovsky krizhanovsky deleted the MekhanikEvgenii/fix-1411-2 branch March 11, 2025 13:07
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.

3 participants