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.

Sign up for GitHub

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

Jump to bottom

[DocDB] ASAN leak in RemoteBootstrapAnchorClient::KeepLogAnchorAliveAsync #16061

Closed
bmatican opened this issue Feb 10, 2023 · 1 comment
Closed

[DocDB] ASAN leak in RemoteBootstrapAnchorClient::KeepLogAnchorAliveAsync #16061

bmatican opened this issue Feb 10, 2023 · 1 comment
Assignees
@basavaraj29
Labels
area/docdb YugabyteDB core features kind/bug This issue is a bug kind/failing-test Tests and testing infra priority/high High Priority

Comments

@bmatican
Copy link
Contributor

bmatican commented Feb 10, 2023 •
edited
Loading

Jira Link: DB-5454

Description

https://detective-gcp.dev.yugabyte.com/job/yugabyte-db-phabricator%2F88142%2Fartifact%2Fbuild%2Fasan-clang15-dynamic-ninja%2Fyb-test-logs%2Ftests-integration-tests__remote_bootstrap-itest%2FRemoteBootstrapITest_TestRemoteBootstrapFromClosestPeer.log?class=RemoteBootstrapITest&name=TestRemoteBootstrapFromClosestPeer

https://detective-gcp.dev.yugabyte.com/job/yugabyte-db-phabricator%2F88142%2Fartifact%2Fbuild%2Fasan-clang15-dynamic-ninja%2Fyb-test-logs%2Ftests-integration-tests__remote_bootstrap-itest%2FRemoteBootstrapITest_TestBootstrapSourceCrashesWhileFetchingData.log?class=RemoteBootstrapITest&max_lines=3000&name=TestBootstrapSourceCrashesWhileFetchingData&start_line=3001

==387510==ERROR: LeakSanitizer: detected memory leaks
2409 | [ts-3] |  
2410 | [ts-3] | Direct leak of 96 byte(s) in 4 object(s) allocated from:
2411 | [ts-3] | #0 0x55d20d24890d in operator new(unsigned long) /opt/yb-build/llvm/yb-llvm-v15.0.3-yb-1-1667030060-0b8d1183-almalinux8-x86_64-build/src/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:95:3
2412 | [ts-3] | #1 0x7f8d5cad6a4c in yb::tserver::RemoteBootstrapAnchorClient::KeepLogAnchorAliveAsync() ${BUILD_ROOT}/../../src/yb/tserver/remote_bootstrap_anchor_client.cc:140:40
2413 | [ts-3] | #2 0x7f8d5cb2db48 in yb::tserver::RemoteBootstrapSession::RefreshRemoteLogAnchorSessionAsync() ${BUILD_ROOT}/../../src/yb/tserver/remote_bootstrap_session.cc:555:5
2414 | [ts-3] | #3 0x7f8d5cb0f709 in yb::tserver::RemoteBootstrapServiceImpl::SessionData::ResetExpiration(yb::tserver::RemoteBootstrapErrorPB_Code*) ${BUILD_ROOT}/../../src/yb/tserver/remote_bootstrap_service.cc:429:26

Seems like a pretty obvious leak, where we new a response, but there's no smart pointers around it's ownership to delete the memory. cc @amitanandaiyer @rthallamko3

This seems like a product leak too! Release wise, this is in 2.16.
@bmatican bmatican added area/docdb YugabyteDB core features status/awaiting-triage Issue awaiting triage labels Feb 10, 2023
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Feb 10, 2023
@bmatican bmatican added kind/failing-test Tests and testing infra priority/high High Priority and removed priority/medium Medium priority issue labels Feb 10, 2023
@yugabyte-ci yugabyte-ci removed the status/awaiting-triage Issue awaiting triage label Feb 10, 2023
@basavaraj29
Copy link
Contributor

Additional Context - This only happens when the flag remote_bootstrap_from_leader_only is explicitly set to false (defaults to true). The leak happens in the tests that we set up for RBS from follower feature.

basavaraj29 pushed a commit that referenced this issue Feb 16, 2023
@basavaraj29
[#16061] DocDB: Fixes memory leak issues in RemoteBootstrapAnchorClient
0c4d5e7 
Summary: This diff addresses the memory leak issues in class RemoteBootstrapAnchorClient, in functions ::UpdateLogAnchorAsync & ::KeepLogAnchorAliveAsync. A response object was being created using operator new and paased on to the UpdateLogAnchorAsync/KeepLogAnchorAliveAsync functions. Changed it to use shared_ptrs instead and passing the shared=ptr as argument to the callback function.

Test Plan:
Jenkins
```
ybd asan --cxx-test integration-tests_remote_bootstrap-itest --gtest_filter RemoteBootstrapITest.TestRemoteBootstrapFromClosestPeer -n 100
ybd asan --cxx-test integration-tests_remote_bootstrap-itest --gtest_filter RemoteBootstrapITest.TestBootstrapSourceCrashesWhileFetchingData -n 100
```

Reviewers: rthallam, yyan, qhu, amitanand

Reviewed By: amitanand

Subscribers: hsunder, ybase, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D22879
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@basavaraj29 basavaraj29

Labels
area/docdb YugabyteDB core features kind/bug This issue is a bug kind/failing-test Tests and testing infra priority/high High Priority
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@bmatican @yugabyte-ci @basavaraj29