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

Fix the zombie solver processes mentioned in #477 #691

Merged
merged 1 commit into from
May 10, 2024

Conversation

lsrcz
Copy link
Collaborator

@lsrcz lsrcz commented Apr 25, 2024

I encountered the same issue as mentioned in #477, and this pull request tries to fix it. I do not fully understand the codebase, so please carefully review my changes.

I believe that the root cause of the issue is that the queryTerminate (i.e., the cleanUp function defined in runSolver) is skipped when an exception happens before it but after the solver has been started by the runSolver call.

This pull request moves the termination of the solver to executeQuery function, and terminates it immediately after we finished all the interaction with the solver. In this function, we have the handy ExtractIO class, which allows us to do the clean up no matter whether an exception is raised with finally.

The ExtractIO instance for IO is also buggy, though. It violates the law mentioned in the comment. This pull request also fixes it, and the fix is required for the clean up to work.

class MonadIO m => ExtractIO m where
-- | Law: the @m a@ yielded by 'IO' is pure with respect to 'IO'.
extractIO :: m a -> IO (m a)

After doing these changes, I observed no zombie or long-running processes after killing a thread running sbv.

@LeventErkok
Copy link
Owner

Great! Unfortunately I’m away from any decent computing device till about May 15th. I’ll take a look at that time.

I’m happy to merge to mainline if you’ve an urgent need. Let me know.

@lsrcz
Copy link
Collaborator Author

lsrcz commented Apr 25, 2024

No hurry, I can rely on my own patched version for now.

@LeventErkok LeventErkok merged commit a83893c into LeventErkok:master May 10, 2024
2 checks passed
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