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

terminate the solver process on stop #13

Merged
merged 1 commit into from
Aug 22, 2022
Merged

terminate the solver process on stop #13

merged 1 commit into from
Aug 22, 2022

Conversation

temyurchenko
Copy link
Contributor

otherwise the solver can hang in the background a very long time

@yav
Copy link
Owner

yav commented Aug 15, 2022

I can see some situations where you just want to kill the solver, but it's probably nice to have a proper exit option too. So I'd suggest we do the following:

  1. replace the filed stop :: IO ExitCode with a different one stopMaybeForce :: Bool -> IO ExitCode
  2. define stopMaybeForce force = if force then terminate else old_beahvior
  3. add a function stop :: IO ExitCode defined as stopMaybeFroce False
  4. add a new function forceStop :: IO ExitCode defined as stopMaybeForce True

What do you think?

@temyurchenko
Copy link
Contributor Author

That makes sense, will do. In that case, it would even be possible to give the solver some timeout to stop gracefully and kill it if it doesn't.

Is there any reason to have specifically stopMaybeForce rather than just stop and forceStop?

@yav
Copy link
Owner

yav commented Aug 15, 2022

Good point, probably simpler to just add forceStop as an extra method.

@jwaldmann
Copy link

I can see some situations where you just want to kill the solver,

I also do: several solver instances are running concurrently (on different constraint systems - that are equivalent w.r.t. my application). I just want the first solution - and kill all other solver instances.

@temyurchenko
Copy link
Contributor Author

Done. If you don't like the terminate/exit naming, I can change it to stop/forceStop.

@yav
Copy link
Owner

yav commented Aug 19, 2022

Hi, thanks for the changes. I think it's a good idea to keep the old one as stop to avoid breaking existing code. I don't mind about the name of the new function (the one that just kills the process), but perhaps stop/forceStop is more obvious than stop/terminate

otherwise the solver can hang in the background a very long time
@temyurchenko
Copy link
Contributor Author

Hi, thanks for the changes. I think it's a good idea to keep the old one as stop to avoid breaking existing code. I don't mind about the name of the new function (the one that just kills the process), but perhaps stop/forceStop is more obvious than stop/terminate

Definitely, what was I thinking

@yav yav merged commit dd03ca1 into yav:master Aug 22, 2022
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.

None yet

3 participants