Skip to content

CURLOPT_PIPEWAIT, allow waited reuse also for subsequent connections.#10456

Closed
icing wants to merge 1 commit intocurl:masterfrom
icing:h2-parallel-use
Closed

CURLOPT_PIPEWAIT, allow waited reuse also for subsequent connections.#10456
icing wants to merge 1 commit intocurl:masterfrom
icing:h2-parallel-use

Conversation

@icing
Copy link
Contributor

@icing icing commented Feb 9, 2023

As tesated in test_02_07, when firing off 200 urls with --parallel, 199 wait for the first connection to be established. if that is multiuse, urls are added up to its capacity.
The first url over capacity opens another connection. But subseqent urls found the same situation and open a connection too. They should have waited for the second connection to actually connect and make its capacity known.

This change fixes that by

  • setting connkeep() early in the HTTP setup handler. as otherwise a new connection is marked as closeit by default and not considered for multiuse at all
  • checking the "connected" status for a candidate always and continuing to PIPEWAIT if no alternative is found.

pytest:

  • removed "skip" from test_02_07
  • added test_02_07b to check that http/1.1 continues to work as before

As tesated in test_02_07, when firing off 200 urls with --parallel,
199 wait for the first connection to be established. if that is multiuse,
urls are added up to its capacity.
The first url over capacity opens another connection. But subseqent
urls found the same situation and open a connection too. They should
have waited for the second connection to actually connect and make
its capacity known.

This change fixes that by

- setting `connkeep()` early in the HTTP setup handler. as otherwise
  a new connection is marked as closeit by default and not considered
  for multiuse at all
- checking the "connected" status for a candidate always and continuing
  to PIPEWAIT if no alternative is found.

pytest:
- removed "skip" from test_02_07
- added test_02_07b to check that http/1.1 continues to work as before
@icing icing requested a review from bagder February 10, 2023 09:23
@bagder
Copy link
Member

bagder commented Feb 10, 2023

I'll play safe and merge this after the 7.88.0 release, to give this a little more time between merge and release.

@bagder bagder closed this in 821f6e2 Feb 21, 2023
@bagder
Copy link
Member

bagder commented Feb 21, 2023

Thanks!

bch pushed a commit to bch/curl that referenced this pull request Jul 19, 2023
As tested in test_02_07, when firing off 200 urls with --parallel, 199
wait for the first connection to be established. if that is multiuse,
urls are added up to its capacity.

The first url over capacity opens another connection. But subsequent
urls found the same situation and open a connection too. They should
have waited for the second connection to actually connect and make its
capacity known.

This change fixes that by

- setting `connkeep()` early in the HTTP setup handler. as otherwise
  a new connection is marked as closeit by default and not considered
  for multiuse at all
- checking the "connected" status for a candidate always and continuing
  to PIPEWAIT if no alternative is found.

pytest:
- removed "skip" from test_02_07
- added test_02_07b to check that http/1.1 continues to work as before

Closes curl#10456
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants