Skip to content

Commit

Permalink
fix(broker/lua): Memory leak due to garbage collector fixed
Browse files Browse the repository at this point in the history
REFS: MON-34712
  • Loading branch information
jean-christophe81 authored Feb 23, 2024
1 parent 11570c5 commit ed6745a
Show file tree
Hide file tree
Showing 30 changed files with 1,908 additions and 357 deletions.
7 changes: 7 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ if(NOT CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND NOT CMAKE_CXX_COMPILER_ID
FATAL_ERROR "You can build broker with g++ or clang++. CMake will exit.")
endif()

option(WITH_MALLOC_TRACE "compile centreon-malloc-trace library." OFF)

# set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++14 -stdlib=libc++")
# set(CMAKE_CXX_COMPILER "clang++")
add_definitions("-D_GLIBCXX_USE_CXX11_ABI=1")
Expand Down Expand Up @@ -185,6 +187,11 @@ add_subdirectory(engine)
add_subdirectory(connectors)
add_subdirectory(ccc)

if (WITH_MALLOC_TRACE)
add_subdirectory(malloc-trace)
endif()


add_custom_target(test-broker COMMAND tests/ut_broker)
add_custom_target(test-engine COMMAND tests/ut_engine)
add_custom_target(test-clib COMMAND tests/ut_clib)
Expand Down
50 changes: 27 additions & 23 deletions broker/core/multiplexing/src/muxer.cc
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
/*
** Copyright 2009-2013,2015-2017,2019-2021 Centreon
**
** Licensed under the Apache License, Version 2.0 (the "License");
** you may not use this file except in compliance with the License.
** You may obtain a copy of the License at
**
** http://www.apache.org/licenses/LICENSE-2.0
**
** Unless required by applicable law or agreed to in writing, software
** distributed under the License is distributed on an "AS IS" BASIS,
** WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
** See the License for the specific language governing permissions and
** limitations under the License.
**
** For more information : [email protected]
*/
/**
* Copyright 2009-2013,2015-2017,2019-2021 Centreon
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* For more information : [email protected]
*/

#include "com/centreon/broker/multiplexing/muxer.hh"

Expand Down Expand Up @@ -195,15 +195,19 @@ std::shared_ptr<muxer> muxer::create(std::string name,
* Destructor.
*/
muxer::~muxer() noexcept {
stats::center::instance().unregister_muxer(_name);
unsubscribe();
std::lock_guard<std::mutex> lock(_mutex);
SPDLOG_LOGGER_INFO(log_v2::core(),
"Destroying muxer {}: number of events in the queue: {}",
_name, _events_size);
_clean();
{
std::lock_guard<std::mutex> lock(_mutex);
SPDLOG_LOGGER_INFO(log_v2::core(),
"Destroying muxer {}: number of events in the queue: {}",
_name, _events_size);
_clean();
}
DEBUG(
fmt::format("DESTRUCTOR muxer {:p} {}", static_cast<void*>(this), _name));
// caution, unregister_muxer must be the last center method called at muxer
// destruction to avoid re create a muxer stat entry
stats::center::instance().unregister_muxer(_name);
}

/**
Expand Down
1 change: 1 addition & 0 deletions broker/core/sql/src/mysql_connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,7 @@ void mysql_connection::_run() {
::mysql_error(_conn)));
_state = finished;
_start_condition.notify_all();
_clear_connection();
return;
}
_last_access = std::time(nullptr);
Expand Down
13 changes: 13 additions & 0 deletions broker/lua/inc/com/centreon/broker/lua/broker_event.hh
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,23 @@ namespace lua {
*
*/
class broker_event {
struct gc_info {
gc_info() : _broker_event_cpt(0), _last_full_gc(time(nullptr)) {}

unsigned _broker_event_cpt;
time_t _last_full_gc;
};

static std::map<const lua_State*, gc_info> _gc_info;
static std::mutex _gc_info_m;

static int l_broker_event_destructor(lua_State* L);

public:
static void broker_event_reg(lua_State* L);
static void create(lua_State* L, std::shared_ptr<io::data> e);
static void create_as_table(lua_State* L, const io::data& e);
static void lua_close(const lua_State* L);
};
} // namespace lua

Expand Down
36 changes: 18 additions & 18 deletions broker/lua/inc/com/centreon/broker/lua/luabinding.hh
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
/*
** Copyright 2018 Centreon
**
** Licensed under the Apache License, Version 2.0 (the "License");
** you may not use this file except in compliance with the License.
** You may obtain a copy of the License at
**
** http://www.apache.org/licenses/LICENSE-2.0
**
** Unless required by applicable law or agreed to in writing, software
** distributed under the License is distributed on an "AS IS" BASIS,
** WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
** See the License for the specific language governing permissions and
** limitations under the License.
**
** For more information : [email protected]
*/
/**
* Copyright 2018-2024 Centreon
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* For more information : [email protected]
*/

#ifndef CCB_LUA_LUABINDING_HH
#define CCB_LUA_LUABINDING_HH
Expand Down Expand Up @@ -112,7 +112,7 @@ class luabinding {
macro_cache& cache);
luabinding(luabinding const&) = delete;
luabinding& operator=(luabinding const&) = delete;
~luabinding() noexcept = default;
~luabinding() noexcept;
bool has_filter() const noexcept;
int32_t write(std::shared_ptr<io::data> const& data) noexcept;
bool has_flush() const noexcept;
Expand Down
47 changes: 46 additions & 1 deletion broker/lua/src/broker_event.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "com/centreon/broker/io/data.hh"
#include "com/centreon/broker/io/protobuf.hh"
#include "com/centreon/broker/mapping/entry.hh"
#include "com/centreon/broker/multiplexing/muxer.hh"
#include "com/centreon/exceptions/msg_fmt.hh"

using namespace com::centreon::broker;
Expand All @@ -32,6 +33,8 @@ static void _write_item(lua_State* L,
const google::protobuf::Message* p,
const google::protobuf::FieldDescriptor* f);

std::map<const lua_State*, broker_event::gc_info> broker_event::_gc_info;
std::mutex broker_event::_gc_info_m;
/**
* The Lua broker_event constructor
*
Expand All @@ -48,6 +51,32 @@ void broker_event::create(lua_State* L, std::shared_ptr<io::data> e) {

luaL_getmetatable(L, "broker_event");
lua_setmetatable(L, -2);
bool have_to_gc_collect = false;
{
std::lock_guard l(_gc_info_m);

/*In V2, lua stores only a userdata that contains a shared_ptr of event
* (16 bytes). So garbage collector don't see amount of memory used by
* events.
* So we need to call garbage collector ourselves to reduce memory
* consumption
* So we call at least gc every minute or
* at most every 10s if lua own more than
* com::centreon::broker::multiplexing::muxer::event_queue_max_size() events
* */
time_t now = time(nullptr);
gc_info& gc_inf = _gc_info[L];
if ((++gc_inf._broker_event_cpt > com::centreon::broker::multiplexing::
muxer::event_queue_max_size() &&
gc_inf._last_full_gc + 10 < now) ||
(gc_inf._last_full_gc + 60 < now)) {
gc_inf._last_full_gc = now;
have_to_gc_collect = true;
}
}
if (have_to_gc_collect) {
lua_gc(L, LUA_GCCOLLECT, 0);
}
}

static void _message_to_table(lua_State* L,
Expand Down Expand Up @@ -344,12 +373,18 @@ void broker_event::create_as_table(lua_State* L, const io::data& d) {
*
* @return 0
*/
static int l_broker_event_destructor(lua_State* L) {
int broker_event::l_broker_event_destructor(lua_State* L) {
void* ptr = luaL_checkudata(L, 1, "broker_event");

if (ptr) {
auto event = static_cast<std::shared_ptr<io::data>*>(ptr);
event->reset();
std::lock_guard l(_gc_info_m);

gc_info& gc_inf = _gc_info[L];
if (gc_inf._broker_event_cpt > 0) {
--gc_inf._broker_event_cpt;
}
}
return 0;
}
Expand Down Expand Up @@ -758,3 +793,13 @@ void broker_event::broker_event_reg(lua_State* L) {

lua_setglobal(L, name);
}

/**
* @brief when a lua_State is closed we clean _gc_info
*
* @param The Lua interpreter as a lua_State*
*/
void broker_event::lua_close(const lua_State* L) {
std::lock_guard l(_gc_info_m);
_gc_info.erase(L);
}
Loading

1 comment on commit ed6745a

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
31 1 0 32 96.88 0s

Failed Tests

Name Message ⏱️ Duration Suite
BEEXTCMD_REVERSE_GRPC4 Central Broker not correctly stopped: -11 != 0 0.000 s External-Commands2

Please sign in to comment.