Skip to content

test/objectstore/test_bluefs: avoid leak of writers#63602

Closed
tchaikov wants to merge 1 commit intoceph:mainfrom
tchaikov:wip-bluefs-test-fix-leak
Closed

test/objectstore/test_bluefs: avoid leak of writers#63602
tchaikov wants to merge 1 commit intoceph:mainfrom
tchaikov:wip-bluefs-test-fix-leak

Conversation

@tchaikov
Copy link
Contributor

@tchaikov tchaikov commented May 30, 2025

previously, we mounted BlueFS for testing, but we didn't umount it after done with the testing. the log.writer was leaked this way.

and ASan rightly pointed this out:

Direct leak of 256 byte(s) in 1 object(s) allocated from:
    #0 0x55dcd54e9a2d in operator new[](unsigned long) (/home/jenkins-build/build/workspace/ceph-pull-requests/build/bin/unittest_bluefs+0x3b7a2d) (BuildId: a8b1c1bb869c4f8f832195839191cf81d3670f77)
    #1 0x55dcd571a5de in mempool::pool_allocator<(mempool::pool_index_t)17, BlueFS::FileWriter>::allocate(unsigned long, void*) /home/jenkins-build/build/workspace/ceph-pull-requests/src/include/mempool.h:353:33
    #2 0x55dcd569c020 in BlueFS::FileWriter::operator new(unsigned long) /home/jenkins-build/build/workspace/ceph-pull-requests/src/os/bluestore/BlueFS.cc:52:1
    #3 0x55dcd56a8e5a in BlueFS::_create_writer(boost::intrusive_ptr<BlueFS::File>) /home/jenkins-build/build/workspace/ceph-pull-requests/src/os/bluestore/BlueFS.cc:4679:19
    #4 0x55dcd56b0ff5 in BlueFS::mount() /home/jenkins-build/build/workspace/ceph-pull-requests/src/os/bluestore/BlueFS.cc:1129:16
    #5 0x55dcd554a97c in BlueFS_test_log_runway_3_Test::TestBody() /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/objectstore/test_bluefs.cc:2185:6
    #6 0x55dcd566012d in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2653:10
    #7 0x55dcd5622095 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2689:14
    #8 0x55dcd55dcddd in testing::Test::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2728:5

in this change, we manually umount the BlueFS at the end of testing to address the leak.

also, we failed to free the created writer, let's free it after using it.

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands

@github-actions github-actions bot added the tests label May 30, 2025
@tchaikov tchaikov mentioned this pull request May 30, 2025
14 tasks
Fix two memory leaks in BlueFS test cases:

1. BlueFS instances were mounted but never unmounted, causing log.writer
   objects to leak. Added explicit umount() calls at test completion.

2. Created FileWriter objects were not being freed after use. Added
   proper cleanup to free writers when no longer needed.

These leaks were detected by AddressSanitizer, which reported a direct
leak in the FileWriter allocation path:

```
Direct leak of 256 byte(s) in 1 object(s) allocated from:
    #0 0x55dcd54e9a2d in operator new[](unsigned long) (/home/jenkins-build/build/workspace/ceph-pull-requests/build/bin/unittest_bluefs+0x3b7a2d) (BuildId: a8b1c1bb869c4f8f832195839191cf81d3670f77)
    #1 0x55dcd571a5de in mempool::pool_allocator<(mempool::pool_index_t)17, BlueFS::FileWriter>::allocate(unsigned long, void*) /home/jenkins-build/build/workspace/ceph-pull-requests/src/include/mempool.h:353:33
    #2 0x55dcd569c020 in BlueFS::FileWriter::operator new(unsigned long) /home/jenkins-build/build/workspace/ceph-pull-requests/src/os/bluestore/BlueFS.cc:52:1
    #3 0x55dcd56a8e5a in BlueFS::_create_writer(boost::intrusive_ptr<BlueFS::File>) /home/jenkins-build/build/workspace/ceph-pull-requests/src/os/bluestore/BlueFS.cc:4679:19
    #4 0x55dcd56b0ff5 in BlueFS::mount() /home/jenkins-build/build/workspace/ceph-pull-requests/src/os/bluestore/BlueFS.cc:1129:16
    #5 0x55dcd554a97c in BlueFS_test_log_runway_3_Test::TestBody() /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/objectstore/test_bluefs.cc:2185:6
    #6 0x55dcd566012d in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2653:10
    #7 0x55dcd5622095 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2689:14
    #8 0x55dcd55dcddd in testing::Test::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2728:5
```

in this change, we manually umount the BlueFS at the end of testing
to address the leak.

also, we failed to free the created writer, let's free it after using it.

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
@tchaikov tchaikov force-pushed the wip-bluefs-test-fix-leak branch from 5e392f2 to 8acd746 Compare May 30, 2025 08:31
@tchaikov
Copy link
Contributor Author

jenkins test windows

@tchaikov tchaikov requested a review from aclamk May 31, 2025 02:54
@tchaikov
Copy link
Contributor Author

@aclamk hi Adam, could you please help review this change?

@tchaikov
Copy link
Contributor Author

tchaikov commented Jun 8, 2025

closing in favor of #63761

@tchaikov tchaikov closed this Jun 8, 2025
@tchaikov tchaikov deleted the wip-bluefs-test-fix-leak branch June 12, 2025 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant