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

Fix BlockingHttp4sServlet handling requests asynchronously (0.21) #2955

Closed

Conversation

nigredo-tori
Copy link
Contributor

@nigredo-tori nigredo-tori commented Nov 5, 2019

See #2939, #2941.

Closes #2939.

@nigredo-tori
Copy link
Contributor Author

nigredo-tori commented Nov 5, 2019

[info] Blaze Http1Client should
...
[error]   x reset request timeout (1 second, 32 ms)
[error]    an exception was thrown Request timeout after 1000 ms (BlazeClientSpec.scala:252)
[error] org.http4s.client.blaze.BlazeClientSpec.$anonfun$new$62(BlazeClientSpec.scala:252)

Interesting. This doesn't look related to this PR, unless I've somehow messed up one of the test threadpools - which is definitely possible. That .evalTap looks wrong to me now - the write is not shifted to blocker, so it happens on the ContextShift - which could explain this.

I'll try and fix this a couple hours later - both here and in 0.20.

@rossabaker
Copy link
Member

It is probably unrelated. We've had flaky blaze tests like this one on Travis for a very long time and not been able to get to the bottom of it. It's also slow to troubleshoot, because it doesn't happen locally. I think it's related to too many unsafeRunSync()s in the tests temporarily locking up the more limited resources of the CI environment. We need to make better use of the async testing features introduced in later specs2.

@rossabaker
Copy link
Member

@nigredo-tori Is the evalTap issue serious enough to delay releasing 0.20.13? We have several other bugfixes people are waiting on, but we can wait a few hours. Otherwise, we can release again real soon.

@nigredo-tori
Copy link
Contributor Author

Is the evalTap issue serious enough to delay releasing 0.20.13?

There's definitely a deadlock possible there in some cases (e.g. in tests if the client is running on the same ContextShift). But it doesn't look like anyone but me is using the blocking servlet, so it's not a blocker (no pun intended).

At any rate, I've fixed this here, and sent a PR against series/0.20 (#2957).

@rossabaker
Copy link
Member

This was handled in the merge from series/0.20.

@rossabaker rossabaker closed this Nov 26, 2019
@nigredo-tori nigredo-tori deleted the 2939-fix-blocking-servlet-0.21 branch November 27, 2019 00:17
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.

BlockingHttp4sContext doesn't properly handle shifted IO
2 participants