-
Notifications
You must be signed in to change notification settings - Fork 57
Close memory leaks on CVC5 and Bitwuzla #585
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
Conversation
We use an approach very similar to what CVC5 is doing in its Java bindings. SWIG uses finalizers to free native memory, however, this makes it very difficult to control the order in which objects are freed. It's also impossible to debug. However, I believe that the crashes were caused by running a finalizer of a Term/Sort object *after* the TermManager was already deleted
We're now deleting Terms when the SolverContext is closed, so they can no longer be accessed in the test
…warnings about possible resource leaks
We would have to wait for the thread to make sure it's not still running when the TermManager is destroyed by the @after method. Since the test seems to be about an old bug that has since been fixed, I'm simply removing it
kfriedberger
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done. Thanks for the analysis and solution.
I will try to find some time soon for updating the solver library.
Side note: Which tool was used to generate the graphs for measurements? JProfiler or similar?
lib/native/source/libbitwuzla/src/org/sosy_lab/java_smt/solvers/bitwuzla/api/TermManager.java
Show resolved
Hide resolved
lib/native/source/libbitwuzla/src/org/sosy_lab/java_smt/solvers/bitwuzla/api/TermManager.java
Show resolved
Hide resolved
Thanks!
I used procpath for the graphs and wrote a small program to repeatably run the same JavaSMT trace so that we can monitor its memory usage. Here is the source for the tool. If needed, this could also be integrated into JavaSMT for benchmarking. The other approach is to use CPA-Daemon to look for memory leaks, but then it's not entirely clear if the problem is with JavaSMT or CPAchecker. Profilers weren't too useful as they can only see the Java heap, and not what is going on in native memory when using JNI |
One more note: This solution works well if there is only a single We could fix this by managing references for each |
Hello,
this PR aims to fix memory leaks on CVC5 and Bitwuzla
Context.deletePointersto make sure all solver memory is freed.Contextis a utility class in the CVC5 JNI bindings that tracks all solver objects. ThedeletePointersmethod can then be called at the end of the program to free native memory. It is not tied to aTermManagerand will delete Objects from all remaining CVC5 SolverContexts, which is why it can only be used when closing the last solver instance.We used to only close
SolverandTermManagerwhen a CVC5 SolverContext is closed. While this approach is more fine-grained, it also leads to memory leaks as Terms and Sorts are never freed, even after theSolverContextis closed. Our new solution is to check inSolverContext.close()if we're closing the last CVC5 instance and then delete all Objects withdeletePointers. Otherwise only theSolverobject is deleted right away, and all other Objects will be collected at the end of the Program. (Thanks to @baierd for the suggestion)Here are some benchmarks to show that the leaks have indeed been closed. The program executes the same JavaSMT trace in a loop while monitoring total memory usage of the process:
CVC5 (before)

CVC5 (after)

Bitwuzla (before)

Bitwuzla (after)

@kfriedberger
We will need to recompile the Bitwuzla bindings for this PR. Could you handle this, please?