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

Better async exception behavior for timeouts #602 #605

Merged
merged 2 commits into from Jan 27, 2017

Conversation

Projects
None yet
2 participants
@snoyberg
Copy link
Member

snoyberg commented Jan 22, 2017

This fixes #602.

Pinging @kazu-yamamoto and @agrafix

@@ -236,7 +236,8 @@ worker :: Context -> S.Settings -> Application -> Responder -> T.Manager -> IO (
worker ctx@Context{inputQ,controlQ} set app responder tm = do
sinfo <- newStreamInfo
tcont <- newThreadContinue
E.bracket (T.registerKillThread tm) T.cancel $ go sinfo tcont
let timeoutAction = return () -- cannot close the shared connection

This comment has been minimized.

@snoyberg

snoyberg Jan 22, 2017

Author Member

@kazu-yamamoto I'm not sure if this is correct, or if there is in fact some way to close the connection when the timeout is triggered.

@kazu-yamamoto

This comment has been minimized.

Copy link
Contributor

kazu-yamamoto commented Jan 23, 2017

I will try to understand this on Friday since I will join the QUIC interim meeting in Tokyo from Tuesday to Thursday.

Before that, it would be nice if we can fix the CI failures above.

@snoyberg

This comment has been minimized.

Copy link
Member Author

snoyberg commented Jan 23, 2017

Hmm, having trouble reproducing locally, investigating.

@snoyberg

This comment has been minimized.

Copy link
Member Author

snoyberg commented Jan 23, 2017

OK, it reproduces reliably on Linux, not at all on OS X. It seems that registering the socket close causes the problem, but I can't see why. Investigation continues.

@snoyberg

This comment has been minimized.

Copy link
Member Author

snoyberg commented Jan 23, 2017

I've narrowed this down to a double-free error in connClose with the call to freeBuffer. However, looking at some tracing output I've added I have no idea how it ever gets called twice. The only other thing I can think of is that the buffer is somehow being freed in a different way elsewhere, such as via GC. If that's the case, I just uncovered a bug in the library somewhere. But this is just conjecture, more likely is that I'm doing something dumb and not realizing it.

Separate connClose and connFree
Thanks to @lehins for figuring out that this was causing the segfault
@snoyberg

This comment has been minimized.

Copy link
Member Author

snoyberg commented Jan 25, 2017

Alright, thanks to @lehins finding the bug, this PR is now passing Travis, and (thankfully) no longer segfaulting :)

@kazu-yamamoto kazu-yamamoto merged commit b11757e into master Jan 27, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@snoyberg snoyberg removed the in progress label Jan 27, 2017

@kazu-yamamoto kazu-yamamoto self-requested a review Jan 27, 2017

@kazu-yamamoto kazu-yamamoto deleted the 602-better-async-timeoutthread branch Jan 27, 2017

@kazu-yamamoto

This comment has been minimized.

Copy link
Contributor

kazu-yamamoto commented Jan 27, 2017

LGTM. Merged.
I will release warp and warp-tls soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.