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

Cleanup code doesn't get run after a tryAny of Snap's httpServe #8

Closed
bitemyapp opened this issue Jun 30, 2015 · 5 comments
Closed

Comments

@bitemyapp
Copy link

This works fine.

        e <- try $ httpServe conf $ site :: IO (Either SomeException ())
        errorM "AdServer.Main.main" ("Got error while serving: " ++ show e)
        cleanup' cleanup

This doesn't run our cleanup code.

        e <- tryAny $ httpServe conf $ site
        errorM "AdServer.Main.main" ("Got error while serving: " ++ show e)
        cleanup' cleanup

I'm a bit flabbergasted as to why a lifted try would cause different behavior here, any ideas?

Repro:

Ctrl-C while blocking on a Snap server in GHCi

Good code runs our cleanup process, output like this:

^C
Shutting down...
Got error while serving: Right ()
Running Snap-provided cleanup function
Shutting down services
Killing the following services: Services {tableUpdate = Just ThreadId 574, stateServer = Just ThreadId 575, ekgServices = Nothing}
Removing logger handlers

Bad code shuts the Snap server down, but none of our secondary services get shut down and none of the logging associated with the cleanup runs. Output like this:

Listening on http://0.0.0.0:8000/
^CInterrupted.
Prelude>
Shutting down...

Prelude>

Only Snap's signal handling is getting run.

What did I do wrong?

@snoyberg
Copy link
Collaborator

If I remember correctly, snap has a broken instance of some of the
typeclasses at play here. I'm on my phone, so can't do more research

On Wed, Jul 1, 2015, 1:22 AM Chris Allen notifications@github.com wrote:

This works fine.

    e <- try $ httpServe conf $ site :: IO (Either SomeException ())
    errorM "AdServer.Main.main" ("Got error while serving: " ++ show e)
    cleanup' cleanup

This doesn't run our cleanup code.

    e <- tryAny $ httpServe conf $ site
    errorM "AdServer.Main.main" ("Got error while serving: " ++ show e)
    cleanup' cleanup

I'm a bit flabbergasted as to why a lifted try would cause different
behavior here, any ideas?

Repro:

Ctrl-C while blocking on a Snap server in GHCi

Good code runs our cleanup process, output like this:

^C
Shutting down...
Got error while serving: Right ()
Running Snap-provided cleanup function
Shutting down services
Killing the following services: Services {tableUpdate = Just ThreadId 574, stateServer = Just ThreadId 575, ekgServices = Nothing}
Removing logger handlers

Bad code shuts the Snap server down, but none of our secondary services
get shut down and none of the logging associated with the cleanup runs.
Output like this:

Listening on http://0.0.0.0:8000/
^CInterrupted.
Prelude>
Shutting down...

Prelude>

Only Snap's signal handling is getting run.

What did I do wrong?


Reply to this email directly or view it on GitHub
#8.

@bitemyapp
Copy link
Author

@snoyberg This is what the Snap people said, but it doesn't really explain to me what's going on here.

15:02 < mightybyte> bitemyapp: No idea what he's talking about there.
15:05 < mightybyte> bitemyapp: Oh, it looks like enclosed-exceptions is using monad-control.  Current snap uses MonadCatchIO-transformers.  That's the only thing I can think of.
15:06 < mightybyte> Snap 1.0 has switched to monad-control though, so perhaps get 1.0 from github and try that?

@snoyberg
Copy link
Collaborator

snoyberg commented Jul 2, 2015

I take it back, I misread the original code. This isn't an issue of listed
versus non lifted. Look at the implementation of tryAny, it's specifically
designed to give different semantics for async exceptions.

On Wed, Jul 1, 2015, 4:32 PM Chris Allen notifications@github.com wrote:

@snoyberg https://github.com/snoyberg This is what the Snap people
said, but it doesn't really explain to me what's going on here.

15:02 < mightybyte> bitemyapp: No idea what he's talking about there.
15:05 < mightybyte> bitemyapp: Oh, it looks like enclosed-exceptions is using monad-control. Current snap uses MonadCatchIO-transformers. That's the only thing I can think of.
15:06 < mightybyte> Snap 1.0 has switched to monad-control though, so perhaps get 1.0 from github and try that?


Reply to this email directly or view it on GitHub
#8 (comment)
.

@bitemyapp
Copy link
Author

@snoyberg bahhh my bad – should've read docstring more carefully. Thank you!

@snoyberg
Copy link
Collaborator

snoyberg commented Jul 2, 2015

For the record, now that I've landed and can think clearly: the broken instance I was referring to is that snap is depending on MonadCatchIO-transformers, and provides a broken instance for Iteratee (it's trivial to break finally with that instance). In the specific use case of Snap, some invariants seem to guarantee that it always works, but it's always concerned me. Based on my misreading of your original report, I had guessed that someone had added a MonadBaseControl instance using the same technique.

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

No branches or pull requests

2 participants