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

103 Early Hints leads to endless wait for headers #7936

Open
stanislaw89 opened this issue Jul 12, 2023 · 5 comments
Open

103 Early Hints leads to endless wait for headers #7936

stanislaw89 opened this issue Jul 12, 2023 · 5 comments
Labels
bug Bug in existing code

Comments

@stanislaw89
Copy link

Steps to reproduce.

Execute the code snipper below multiple times (it reproduces in ~ 1/4 of runs) using v4.11.0

fun main() {
  val client = OkHttpClient().newBuilder()
    .connectTimeout(5000, TimeUnit.MILLISECONDS)
    .readTimeout(5000, TimeUnit.MILLISECONDS)
    .writeTimeout(5000, TimeUnit.MILLISECONDS)
    .callTimeout(10000, TimeUnit.MILLISECONDS)
    .build()

  val request = Request.Builder()
    .url("https://theornateoracle.com/cart.php?action=add&product_id=456")
    .addHeader(
      "User-Agent", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) " +
      "AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.143 Safari/537.36"
    )
    .cacheControl(CacheControl.FORCE_NETWORK)
    .get()
    .build()

  val call = client.newCall(request)
  println("Executing...")
  call.execute()
  println("Done")
}

The main thread hangs forever in waitForIo of Http2Stream#takeHeaders.
I investigated a bit, and it looks like it only happens for 103 Early Hints response codes (inside the CallServerInterceptor#shouldIgnoreAndWaitForRealResponse if).
Also, I figured out that the Http2Stream.StreamTimeout is called after some time, but it does not unblock the main thread.

@stanislaw89 stanislaw89 added the bug Bug in existing code label Jul 12, 2023
@yschimke
Copy link
Collaborator

Yep - there is a race condition, and when it blocks we are treating them as trailers. I'll try to find the right fix.

@yschimke
Copy link
Collaborator

I could reproduce with 5.x, so if blocked use that until a release.

@yschimke
Copy link
Collaborator

I haven't reproduced on 5.x, and that server now doesn't return a 103.

I have some alternate servers, but I think this particular error relates to a 103 and then a response without a body.

https://early-hints.fastlylabs.com/test.png
https://tradingstrategy.ai

So I don't have a good repro to confirm the fix.

@yschimke
Copy link
Collaborator

@swankjesse I think I understand why it's only happening when the response body is empty.

  1. timeout - which calls closeInternal
  2. closeInternal short circuits because source.finished == true.
  3. takeHeaders loops another time, but now without a timeout

@yschimke
Copy link
Collaborator

Minimal fix in #8054

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug in existing code
Projects
None yet
Development

No branches or pull requests

2 participants