From 26b39f1eae1f576669cbf49c6db94ef4ed8624df Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Tue, 24 Sep 2019 19:05:28 +0200 Subject: [PATCH] Fix some reference counters (memleak) (#8981) Fix some reference counters (memleak) Map::dispatchEvent: Allocation safety using references --- src/client/fontengine.cpp | 5 +---- src/gui/guiFormSpecMenu.cpp | 6 ++++-- src/map.cpp | 8 ++++---- src/map.h | 17 +++-------------- src/mapgen/mg_schematic.cpp | 2 +- src/mapgen/treegen.cpp | 2 +- src/rollback_interface.cpp | 2 +- src/script/lua_api/l_env.cpp | 4 ++-- src/script/lua_api/l_nodemeta.cpp | 2 +- src/script/lua_api/l_vmanip.cpp | 2 +- src/server.cpp | 15 ++++++++++----- src/server.h | 2 +- 12 files changed, 30 insertions(+), 37 deletions(-) diff --git a/src/client/fontengine.cpp b/src/client/fontengine.cpp index c32d0f1a4..858d6780e 100644 --- a/src/client/fontengine.cpp +++ b/src/client/fontengine.cpp @@ -275,7 +275,6 @@ void FontEngine::initFont(unsigned int basesize, FontMode mode) font_shadow_alpha); if (font) { - font->grab(); m_font_cache[mode][basesize] = font; return; } @@ -365,8 +364,6 @@ void FontEngine::initSimpleFont(unsigned int basesize, FontMode mode) } } - if (font) { - font->grab(); + if (font) m_font_cache[mode][basesize] = font; - } } diff --git a/src/gui/guiFormSpecMenu.cpp b/src/gui/guiFormSpecMenu.cpp index e8a7f546e..390c81bc1 100644 --- a/src/gui/guiFormSpecMenu.cpp +++ b/src/gui/guiFormSpecMenu.cpp @@ -1208,12 +1208,14 @@ void GUIFormSpecMenu::createTextField(parserData *data, FieldSpec &spec, true, Environment, this, spec.fid, rect, is_editable, is_multiline); e->drop(); } else { - if (is_multiline) + if (is_multiline) { e = new GUIEditBoxWithScrollBar(spec.fdefault.c_str(), true, Environment, this, spec.fid, rect, is_editable, true); - else if (is_editable) + e->drop(); + } else if (is_editable) { e = Environment->addEditBox(spec.fdefault.c_str(), rect, true, this, spec.fid); + } } if (e) { diff --git a/src/map.cpp b/src/map.cpp index 022eb9f19..0a7099a06 100644 --- a/src/map.cpp +++ b/src/map.cpp @@ -89,7 +89,7 @@ void Map::removeEventReceiver(MapEventReceiver *event_receiver) m_event_receivers.erase(event_receiver); } -void Map::dispatchEvent(MapEditEvent *event) +void Map::dispatchEvent(const MapEditEvent &event) { for (MapEventReceiver *event_receiver : m_event_receivers) { event_receiver->onMapEditEvent(event); @@ -274,7 +274,7 @@ bool Map::addNodeWithEvent(v3s16 p, MapNode n, bool remove_metadata) succeeded = false; } - dispatchEvent(&event); + dispatchEvent(event); return succeeded; } @@ -299,7 +299,7 @@ bool Map::removeNodeWithEvent(v3s16 p) succeeded = false; } - dispatchEvent(&event); + dispatchEvent(event); return succeeded; } @@ -2220,7 +2220,7 @@ MapBlock* ServerMap::loadBlock(v3s16 blockpos) for (it = modified_blocks.begin(); it != modified_blocks.end(); ++it) event.modified_blocks.insert(it->first); - dispatchEvent(&event); + dispatchEvent(event); } } return block; diff --git a/src/map.h b/src/map.h index 67e00c6f4..392ec3f25 100644 --- a/src/map.h +++ b/src/map.h @@ -79,18 +79,7 @@ struct MapEditEvent MapEditEvent() = default; - MapEditEvent * clone() - { - MapEditEvent *event = new MapEditEvent(); - event->type = type; - event->p = p; - event->n = n; - event->modified_blocks = modified_blocks; - event->is_private_change = is_private_change; - return event; - } - - VoxelArea getArea() + VoxelArea getArea() const { switch(type){ case MEET_ADDNODE: @@ -125,7 +114,7 @@ class MapEventReceiver { public: // event shall be deleted by caller after the call. - virtual void onMapEditEvent(MapEditEvent *event) = 0; + virtual void onMapEditEvent(const MapEditEvent &event) = 0; }; class Map /*: public NodeContainer*/ @@ -152,7 +141,7 @@ public: void addEventReceiver(MapEventReceiver *event_receiver); void removeEventReceiver(MapEventReceiver *event_receiver); // event shall be deleted by caller after the call. - void dispatchEvent(MapEditEvent *event); + void dispatchEvent(const MapEditEvent &event); // On failure returns NULL MapSector * getSectorNoGenerateNoLock(v2s16 p2d); diff --git a/src/mapgen/mg_schematic.cpp b/src/mapgen/mg_schematic.cpp index 36f1dd76b..c1acbfd9d 100644 --- a/src/mapgen/mg_schematic.cpp +++ b/src/mapgen/mg_schematic.cpp @@ -246,7 +246,7 @@ void Schematic::placeOnMap(ServerMap *map, v3s16 p, u32 flags, for (it = modified_blocks.begin(); it != modified_blocks.end(); ++it) event.modified_blocks.insert(it->first); - map->dispatchEvent(&event); + map->dispatchEvent(event); } diff --git a/src/mapgen/treegen.cpp b/src/mapgen/treegen.cpp index 4c351fcef..0d8af2851 100644 --- a/src/mapgen/treegen.cpp +++ b/src/mapgen/treegen.cpp @@ -135,7 +135,7 @@ treegen::error spawn_ltree(ServerEnvironment *env, v3s16 p0, event.type = MEET_OTHER; for (auto &modified_block : modified_blocks) event.modified_blocks.insert(modified_block.first); - map->dispatchEvent(&event); + map->dispatchEvent(event); return SUCCESS; } diff --git a/src/rollback_interface.cpp b/src/rollback_interface.cpp index 0c8aef9ab..c00206e98 100644 --- a/src/rollback_interface.cpp +++ b/src/rollback_interface.cpp @@ -176,7 +176,7 @@ bool RollbackAction::applyRevert(Map *map, InventoryManager *imgr, IGameDef *gam MapEditEvent event; event.type = MEET_BLOCK_NODE_METADATA_CHANGED; event.p = p; - map->dispatchEvent(&event); + map->dispatchEvent(event); } catch (InvalidPositionException &e) { infostream << "RollbackAction::applyRevert(): " << "InvalidPositionException: " << e.what() diff --git a/src/script/lua_api/l_env.cpp b/src/script/lua_api/l_env.cpp index 34e079af7..a56b1cb0b 100644 --- a/src/script/lua_api/l_env.cpp +++ b/src/script/lua_api/l_env.cpp @@ -1035,7 +1035,7 @@ int ModApiEnvMod::l_fix_light(lua_State *L) for (auto &modified_block : modified_blocks) event.modified_blocks.insert(modified_block.first); - map.dispatchEvent(&event); + map.dispatchEvent(event); } lua_pushboolean(L, success); @@ -1144,7 +1144,7 @@ int ModApiEnvMod::l_delete_area(lua_State *L) } } - map.dispatchEvent(&event); + map.dispatchEvent(event); lua_pushboolean(L, success); return 1; } diff --git a/src/script/lua_api/l_nodemeta.cpp b/src/script/lua_api/l_nodemeta.cpp index 22fc61782..229ce73db 100644 --- a/src/script/lua_api/l_nodemeta.cpp +++ b/src/script/lua_api/l_nodemeta.cpp @@ -68,7 +68,7 @@ void NodeMetaRef::reportMetadataChange(const std::string *name) event.type = MEET_BLOCK_NODE_METADATA_CHANGED; event.p = m_p; event.is_private_change = name && meta && meta->isPrivate(*name); - m_env->getMap().dispatchEvent(&event); + m_env->getMap().dispatchEvent(event); } // Exported functions diff --git a/src/script/lua_api/l_vmanip.cpp b/src/script/lua_api/l_vmanip.cpp index c92983bd3..fd73d21d1 100644 --- a/src/script/lua_api/l_vmanip.cpp +++ b/src/script/lua_api/l_vmanip.cpp @@ -126,7 +126,7 @@ int LuaVoxelManip::l_write_to_map(lua_State *L) for (const auto &modified_block : o->modified_blocks) event.modified_blocks.insert(modified_block.first); - map->dispatchEvent(&event); + map->dispatchEvent(event); o->modified_blocks.clear(); return 0; diff --git a/src/server.cpp b/src/server.cpp index c0ed02b9f..0e677199a 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -306,6 +306,11 @@ Server::~Server() for (auto &detached_inventory : m_detached_inventories) { delete detached_inventory.second; } + + while (!m_unsent_map_edit_queue.empty()) { + delete m_unsent_map_edit_queue.front(); + m_unsent_map_edit_queue.pop(); + } } void Server::init() @@ -1090,12 +1095,12 @@ void Server::setTimeOfDay(u32 time) m_time_of_day_send_timer = 0; } -void Server::onMapEditEvent(MapEditEvent *event) +void Server::onMapEditEvent(const MapEditEvent &event) { - if (m_ignore_map_edit_events_area.contains(event->getArea())) + if (m_ignore_map_edit_events_area.contains(event.getArea())) return; - MapEditEvent *e = event->clone(); - m_unsent_map_edit_queue.push(e); + + m_unsent_map_edit_queue.push(new MapEditEvent(event)); } Inventory* Server::getInventory(const InventoryLocation &loc) @@ -1160,7 +1165,7 @@ void Server::setInventoryModified(const InventoryLocation &loc) MapEditEvent event; event.type = MEET_BLOCK_NODE_METADATA_CHANGED; event.p = loc.p; - m_env->getMap().dispatchEvent(&event); + m_env->getMap().dispatchEvent(event); } break; case InventoryLocation::DETACHED: diff --git a/src/server.h b/src/server.h index f01733179..d61840871 100644 --- a/src/server.h +++ b/src/server.h @@ -189,7 +189,7 @@ public: This is accessed by the map, which is inside the environment, so it shouldn't be a problem. */ - void onMapEditEvent(MapEditEvent *event); + void onMapEditEvent(const MapEditEvent &event); /* Shall be called with the environment and the connection locked. -- 2.25.1