Fix memory leak caused by impl.go call libzkp #57
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
FletcherMan
reviewed
Mar 18, 2024
| defer func() { | ||
| C.free(unsafe.Pointer(tracesStr)) | ||
| }() | ||
| defer C.free(unsafe.Pointer(tracesStr)) |
There was a problem hiding this comment.
how does this change make it different?
There was a problem hiding this comment.
There are some subtle differences, I mainly refer to scroll's update
|
同步一下进展：
logs for leak-checking by valgrind通过上面可以看到，libzkp.so 确实是导致了一部分的 possibly lost 和 still reachable，但是不是大头，更多的是因为 secp256k1.c 导致的。然后 libzkp.so 的 log 显示，主要是 env_logger 以及 ccc 的 init, 还有是 hashbrown（就是 std 里面HashMap 用到的库。可能因为 valgrind 的模拟执行根本没有执行到 apply_tx/apply_block 的逻辑。
pgrep geth
# xxx
# use interval of 30 seconds and prune any allocations newer than 5000ms
memleak-bpfcc -p xxx -o 5000 30能检测到 libzkp.so 的一些东西。这里 提到了是因为
不过把 ubuntu:20.04 改成 ubuntu:bionic 仍然有无法在 container 里面正常运行。既然上面有人在 docker 里面成功运行了（他也说了花了他几个小时安装依赖包之类的），因此这个问题是可以解决的。 参考这个修复 go 调用 ZooKeeper C 库导致的内存泄漏的问题： https://github.com/Shopify/gozk/pull/4/files impl.go 里面需要释放的 C 指针都已经使用 defer C.free()/C.free_c_char() 正确释放了。 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ref: scroll-tech/scroll#998
1. Purpose or design rationale of this PR
...
2. PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
3. Deployment tag versioning
Has the version in
params/version.gobeen updated?4. Breaking change label
Does this PR have the
breaking-changelabel?