You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kvrocks.apache.org by GitBox <gi...@apache.org> on 2022/05/20 05:40:08 UTC
[GitHub] [incubator-kvrocks] PragmaTwice opened a new pull request, #599: Improve using of ASan and TSan in CMake build
PragmaTwice opened a new pull request, #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599
Close #590.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1133909443
Copy that, thanks @PragmaTwice.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1144429808
> @PragmaTwice shall we merge master, running CI with nightly changes first, or current status is already ready for review?
Yeah, I think this PR is ready for review now~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] ShooterIT commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
ShooterIT commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1132504385
Thanks @PragmaTwice Cool, I want to this commit too long time.
> `./unittest` aborted while executed with AddressSanitizer:
>
> ```
> SUMMARY: AddressSanitizer: 11268648 byte(s) leaked in 67853 allocation(s).
> ```
Did TCL tests report errors?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1132501162
`./unittest` aborted while executed with AddressSanitizer:
```
SUMMARY: AddressSanitizer: 11268648 byte(s) leaked in 67853 allocation(s).
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1132900063
Those memory leaks Looks all came from Lua scripting, I found we didn't close Lua VM before stopping the server. I will have a try and fix it soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1144488788
@ShooterIT so far, sanitizers run daily and there will be failure reports in "Actions" page if daily CI failed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1138694325
> @PragmaTwice since #605 merged, is this PR ready for review (merge)?
Leaks in unit tests are fixed, but these are still several leaks in TCL tests.
I will investigate it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1143108543
Blocked by #614.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1133901449
@PragmaTwice How can I reproduce the memory leaks in TCL tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1144392444
@PragmaTwice shall we merge master, running CI with nightly changes first, or current status is already ready for review?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1144584625
> One problem though is that the TCL tests seem not to pay attention to the return value of the redis server (kvrocks) when it crashes, which may make the sanitizer's error difficult to observe by developers, I will investigate this issue later.
This seems a separated issue. Merging this PR ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1144528347
One problem though is that the TCL tests seem not to pay attention to the return value when the redis server crashes, which may make the sanitizer's error difficult to observe by developers, I will investigate this issue later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1143690041
#614 is resolved now 🎉
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1138661998
@PragmaTwice since #605 merged, is this PR ready for review (merge)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1132515007
```
$ rg AddressSanitizer .
./server.605939.3/stderr
137:SUMMARY: AddressSanitizer: 684 byte(s) leaked in 8 allocation(s).
```
Seems much less than unit tests?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1132910638
seems like some of leaks from unittest is due to missing the dtor call (i.e. delete operator) after `Storage::Open`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1133908094
> @PragmaTwice How can I reproduce the memory leaks in tcl tests? I have tried to build with `ENABLE_ASAN` and run tcl tests, but can't find leaks error.
Did your tcl tests pass or fail?
There are some suggestions you could try:
- make sure that your `./build/kvrocks` is built with `-DENABLE_ASAN=ON` and from this branch
- execute `runtest` with `--dont-clean`, so you can view stdout/stderr of kvrocks after test (`grep AddressSanitizer . -r` in `./tests/tcl`)
- I am using GCC 11.2 and Clang 13, and they both can make the test fail, so you could try some higher version.
stderr from Clang:
```
$ rg AddressSanitizer .
./tests/tmp.bak/server.605939.3/stderr
137:SUMMARY: AddressSanitizer: 684 byte(s) leaked in 8 allocation(s).
./tests/tmp/server.724500.3/stderr
2:==726298==ERROR: AddressSanitizer: heap-use-after-free on address 0x608000020068 at pc 0x56231866985d bp 0x7fc7b4180570 sp 0x7fc7b4180568
81:SUMMARY: AddressSanitizer: heap-use-after-free /home/twice/incubator-kvrocks/src/redis_cmd.cc:1531:25 in Redis::CommandBPop::TryPopFromList()
./tests/tmp/server.724501.7/stderr
137:SUMMARY: AddressSanitizer: 684 byte(s) leaked in 8 allocation(s).
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun merged pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
tisonkun merged PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1144524818
> Truly thank you @PragmaTwice I waited too long time for this commit.
>
> But i still want to make sure these sanitizers work well, can they report errors if we leak memory or data race access designedly
Yeah, I think these sanitizers work well.
Actually we have fixed two memory leaks found by the LeakSanitizer (#605 and #614).
Maybe later we can add more [matrix strategies](https://docs.github.com/en/actions/using-jobs/using-a-matrix-for-your-jobs) to CI so that both gcc and clang sanitizers can be used in tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1132518286
But a weird thing is that the failed redis-server reported by tcl test has an empty stderr file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #599: Improve using of ASan and TSan in CMake build
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #599:
URL: https://github.com/apache/incubator-kvrocks/pull/599#issuecomment-1132506724
> Thanks @PragmaTwice Cool, I want to this commit too long time.
>
> > `./unittest` aborted while executed with AddressSanitizer:
> > ```
> > SUMMARY: AddressSanitizer: 11268648 byte(s) leaked in 67853 allocation(s).
> > ```
>
> Did TCL tests report errors?
Seems like the server aborted by asan while running tcl tests, I will check its log.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org