Skip to content

don't launch if server fails; kill server on exit #537 #541

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

Merged
merged 2 commits into from
May 2, 2016
Merged

don't launch if server fails; kill server on exit #537 #541

merged 2 commits into from
May 2, 2016

Conversation

simonmichael
Copy link
Contributor

Make the server thread and launch/monitor thread more aware of each
other's failures, using async. Now if the server fails to start we won't
launch a browser. Also when the main thread terminates (eg from ctrl-c
in GHCI), we make sure the server thread does too. There is now a 0.1s
delay before launching the browser, which also helps ensure the server
is ready to serve.

@snoyberg
Copy link
Member

I don't like relying on timing assumptions to make this work. Warp has a setting to run some code after initialization but before the main loop, which would allow handling this more robustly

@simonmichael
Copy link
Contributor Author

Great! PR updated.


-- launch the browser, after waiting until the server
-- signals that it's ready (or raises a startup exception)
let wait = do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use an IORef here? If you use an MVar, then you can get blocking behavior and not need to worry about polling.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfamiliarity/doubt about exception handling. It seems to work, PR updated...

Make the server thread and launch/monitor thread more aware of each
other, using async. Now, we won't launch a browser until the server is
ready to serve pages, and we won't launch it at all if the server fails
to start. Also we make sure to terminate the server thread whenever the
main thread terminates, eg from ctrl-c in GHCI.
@simonmichael
Copy link
Contributor Author

Nice! I appreciate the review.

@snoyberg
Copy link
Member

snoyberg commented May 2, 2016

Alright, I'm out of ideas :). I'll wait for Travis to check before merging, but LGTM.

@snoyberg snoyberg merged commit 2a755b7 into yesodweb:master May 2, 2016
snoyberg added a commit that referenced this pull request May 11, 2016
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.

2 participants