Skip to content

Commit

Permalink
Add support for event loop to over come memory leak errors by CI
Browse files Browse the repository at this point in the history
```
Changes Added : Added event loop support in test code

problem : After adding support for sending events from phosphor-user-manager it is obsderved that it is calling sendEvent which internally calls async_send_handler and allocates memeory for context
since the event loop is not present in test code, callback is never called and
the CI was throwing memory leak error

Direct leak of 1 byte(s) in 1 object(s) allocated from:
    #0 0x7ffa787b91e7 in operator new(unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:99
    openbmc#1 0x7ffa77ad6383 in operator() /usr/local/include/sdbusplus/asio/detail/async_send_handler.hpp:40
    openbmc#2 0x7ffa77ad6383 in async_send<sdbusplus::asio::connection::async_method_call_timed<phosphor::logging::sendEvent(phosphor::logging::MESSAGE_TYPE, sdbusplus::xyz::openbmc_project::Logging::server::Entry::Level, const std::vector<std::__cxx11::basic_string<char> >&, const string&)::<lambda(boost::system::error_code)>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > >
    (phosphor::logging::sendEvent(phosphor::logging::MESSAGE_TYPE, sdbusplus::xyz::openbmc_project::Logging::server::Entry::Level, const std::vector<std::__cxx11::basic_string<char> >&, const string&)::<lambda(boost::system::error_code)>&&, const string&, const string&, const string&, const string&, uint64_t, const std::__cxx11::basic_string<char>&, const std::__cxx11::basic_string<char>&, const std::map<std::__cxx11::basic_string<char>, std::__cxx11::basic_string<char> >&)::<lambda(boost::system::error_code, sdbusplus::message_t&)> > /usr/local/include/sdbusplus/asio/connection.hpp:98
    openbmc#3 0x7ffa77ad6383 in async_method_call_timed<phosphor::logging::sendEvent(phosphor::logging::MESSAGE_TYPE, sdbusplus::xyz::openbmc_project::Logging::server::Entry::Level, const std::vector<std::__cxx11::basic_string<char> >&, const string&)::<lambda(boost::system::error_code)>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > > /usr/local/include/sdbusplus/asio/connection.hpp:192
    openbmc#4 0x7ffa77ad6383 in async_method_call<phosphor::logging::sendEvent(phosphor::logging::MESSAGE_TYPE, sdbusplus::xyz::openbmc_project::Logging::server::Entry::Level, const std::vector<std::__cxx11::basic_string<char> >&, const string&)::<lambda(boost::system::error_code)>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > > /usr/local/include/sdbusplus/asio/connection.hpp:221
    openbmc#5 0x7ffa77ad6383 in phosphor::logging::sendEvent(phosphor::logging::MESSAGE_TYPE, sdbusplus::xyz::openbmc_project::Logging::server::Entry::Level, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) ../lib/redfish_event_log.cpp:112
    openbmc#6 0x55dfba9084a3 in phosphor::certs::Manager::replaceCertificate(phosphor::certs::Certificate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) ../certs_manager.cpp:493
    openbmc#7 0x55dfba8ab09d in phosphor::certs::Certificate::replace(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) ../certificate.cpp:315
    openbmc#8 0x55dfba7981e0 in TestBody ../test/certs_manager_test.cpp:677
    openbmc#9 0x7ffa786e3f2e in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /googletest-662fe38e44900c007eccb65a5d2ea19df7bd520e/googletest/src/gtest.cc:2607
    openbmc#10 0x7ffa786e3f2e in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /googletest-662fe38e44900c007eccb65a5d2ea19df7bd520e/googletest/src/gtest.cc:2643

```

Solution : The memory leak error was thrown because the
memory allocated by "async_send_handler" in sdbusplus was not getting de-allocated
because the callback is never getting called called since there was no event loop
present in test code.

Added event loop support in test code

Fixes jira https://jirasw.nvidia.com/browse/DGXOPENBMC-8881
  • Loading branch information
Chandramohan Harkude committed Jan 16, 2024
1 parent 5a9927f commit ef4c4d6
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 2 deletions.
29 changes: 27 additions & 2 deletions test/ldap_config_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,21 @@ class TestLDAPConfig : public testing::Test
fs.close();
}

void eventLoop(uint8_t numberOfTimes)
{
if (numberOfTimes == 0 || numberOfTimes > 15)
{
return;
}

for (int i = 0; i < numberOfTimes; i++)
{
bus.process_discard();
// wait for 1 seconds
bus.wait(1 * 1000000);
}
}

void TearDown() override
{
fs::remove_all(dir);
Expand Down Expand Up @@ -461,7 +476,8 @@ TEST_F(TestLDAPConfig, testSearchScope)
"MyLdap12", ldap_base::Create::SearchScope::sub,
ldap_base::Create::Type::ActiveDirectory, "attr1", "attr2");
managerPtr->getADConfigPtr()->enabled(true);

// Process D-Bus calls
eventLoop(6);
// Change LDAP SearchScope
managerPtr->getADConfigPtr()->ldapSearchScope(
ldap_base::Config::SearchScope::one);
Expand Down Expand Up @@ -618,7 +634,8 @@ TEST_F(TestLDAPConfig, ConditionalEnableConfig)

EXPECT_EQ(managerPtr->getADConfigPtr()->enabled(), true);
EXPECT_EQ(managerPtr->getOpenLdapConfigPtr()->enabled(), false);

// Process D-Bus calls
eventLoop(5);
// AS AD is already enabled so openldap can't be enabled.
EXPECT_THROW(
{
Expand All @@ -641,6 +658,8 @@ TEST_F(TestLDAPConfig, ConditionalEnableConfig)
EXPECT_EQ(managerPtr->getOpenLdapConfigPtr()->enabled(), false);
// Now enable the openldap
managerPtr->getOpenLdapConfigPtr()->enabled(true);
// Process D-Bus calls
eventLoop(5);
EXPECT_EQ(managerPtr->getOpenLdapConfigPtr()->enabled(), true);
EXPECT_EQ(managerPtr->getADConfigPtr()->enabled(), false);

Expand Down Expand Up @@ -678,6 +697,8 @@ TEST_F(TestLDAPConfig, createPrivMapping)
}
},
PrivilegeMappingExists);
// Process D-Bus calls
eventLoop(2);
}

TEST_F(TestLDAPConfig, deletePrivMapping)
Expand Down Expand Up @@ -719,6 +740,8 @@ TEST_F(TestLDAPConfig, deletePrivMapping)
EXPECT_NO_THROW(manager.getADConfigPtr()->checkPrivilegeMapper("admin"));
manager.getADConfigPtr()->deletePrivilegeMapper(2);
EXPECT_NO_THROW(manager.getADConfigPtr()->checkPrivilegeMapper("user"));
// Process D-Bus calls
eventLoop(2);
}

TEST_F(TestLDAPConfig, restorePrivMapping)
Expand Down Expand Up @@ -798,6 +821,8 @@ TEST_F(TestLDAPConfig, testPrivileges)

EXPECT_NO_THROW(entry->privilege("priv-operator"));
EXPECT_NO_THROW(entry->privilege("priv-user"));
// Process D-Bus calls
eventLoop(5);
}

} // namespace ldap
Expand Down
16 changes: 16 additions & 0 deletions test/ldap_mapper_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,20 @@ class TestSerialization : public testing::Test
char tempDir[] = "/tmp/privmapper_test.XXXXXX";
dir = std::filesystem::path(mkdtemp(tempDir));
}
void eventLoop(uint8_t numberOfTimes)
{
if (numberOfTimes == 0 || numberOfTimes > 15)
{
return;
}

for (int i = 0; i < numberOfTimes; i++)
{
bus.process_discard();
// wait for 1 seconds
bus.wait(1 * 1000000);
}
}

void TearDown() override
{
Expand Down Expand Up @@ -139,6 +153,8 @@ TEST_F(TestSerialization, testValidPrivilege)

EXPECT_NO_THROW(entry->privilege("priv-operator"));
EXPECT_NO_THROW(entry->privilege("priv-user"));
// Process D-Bus calls
eventLoop(5);
}

TEST_F(TestSerialization, testInvalidPrivilege)
Expand Down

0 comments on commit ef4c4d6

Please sign in to comment.