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.

Sign up for GitHub

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

Jump to bottom

Use Lwt_switch instead of Lwt_condition to stop the server loop #352

Open
dinosaure opened this issue Nov 30, 2020 · 0 comments
Open

Use Lwt_switch instead of Lwt_condition to stop the server loop #352

dinosaure opened this issue Nov 30, 2020 · 0 comments
Assignees
@craigfe

Comments

@dinosaure
Copy link
Member

See #339, /cc @talex5:

Hmm, there is a Conduit_lwt.serve. But it returns a Lwt_condition.t to stop it. That's racy - if you broadcast on stop before the loop calls wait then it will miss the event! And it really shouldn't be calling wait each time around the loop.

The normal Lwt way to do that would be to take Lwt_switch argument and abort the loop when the switch is turned off.
A switch can only be turned off once and so you can't miss the event. Also, the switch argument can be optional in the common case where you never want to stop.
@craigfe craigfe self-assigned this Dec 3, 2020
craigfe added a commit to craigfe/ocaml-conduit that referenced this issue Dec 4, 2020
@craigfe
Avoid shutdown race condition on service startup
d7ff4f0 
Fix mirage#352.

The `lwt` and `async` service loops previously took condition variables
that are used to signal when to shutdown. This resulted in a race
condition: the shutdown signal can be broadcast before the server is
waiting on it.

Fixed by having the service loops take switches as configuration
instead.
craigfe added a commit to craigfe/ocaml-conduit that referenced this issue Dec 4, 2020
@craigfe
Avoid shutdown race condition on service startup
c6ac3af 
Fix mirage#352.

The `lwt` and `async` service loops previously took condition variables
that are used to signal when to shutdown. This resulted in a race
condition: the shutdown signal can be broadcast before the server is
waiting on it.

Fixed by having the service loops take switches as configuration
instead.
@craigfe craigfe mentioned this issue Dec 4, 2020
Closed
craigfe added a commit to craigfe/ocaml-conduit that referenced this issue Dec 4, 2020
@craigfe
Avoid shutdown race condition on service startup
2a0d559 
Fix mirage#352.

The `lwt` and `async` service loops previously took condition variables
that were used as a shutdown signal. This resulted in a race condition:
the shutdown signal can be broadcast before the server is waiting on it.
Fixed by having the service loops take switches as configuration
instead.
craigfe added a commit to craigfe/ocaml-conduit that referenced this issue Dec 4, 2020
@craigfe
Avoid shutdown race condition on service startup
e427423 
Fix mirage#352.

The `lwt` and `async` service loops previously took condition variables
that were used as a shutdown signal. This resulted in a race condition:
the shutdown signal can be broadcast before the server is waiting on it.
Fixed by having the service loops take switches as configuration
instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@craigfe craigfe

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Avoid shutdown race condition on service startup craigfe/ocaml-conduit
2 participants
@dinosaure @craigfe