Fix Memory Leak from Npc events (#4483)

--------- Co-authored-by: yamaken93 <1008865+yamaken93@users.noreply.github.com>
otland · Jun 22, 2023 · 2a1bad7 · 2a1bad7
1 parent 8789781
commit 2a1bad7
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 33 deletions.
diff --git a/src/npc.cpp b/src/npc.cpp
@@ -13,7 +13,6 @@ extern Game g_game;
 extern LuaEnvironment g_luaEnvironment;
 
 uint32_t Npc::npcAutoID = 0x20000000;
-NpcScriptInterface* Npc::scriptInterface = nullptr;
 
 void Npcs::reload()
 {
@@ -22,9 +21,6 @@ void Npcs::reload()
 		it.second->closeAllShopWindows();
 	}
 
-	delete Npc::scriptInterface;
-	Npc::scriptInterface = nullptr;
-
 	for (const auto& it : npcs) {
 		it.second->reload();
 	}
@@ -39,8 +35,7 @@ Npc* Npc::createNpc(const std::string& name)
 	return npc.release();
 }
 
-Npc::Npc(const std::string& name) :
-    Creature(), filename("data/npc/" + name + ".xml"), npcEventHandler(nullptr), masterRadius(-1), loaded(false)
+Npc::Npc(const std::string& name) : Creature(), filename("data/npc/" + name + ".xml"), masterRadius(-1), loaded(false)
 {
 	reset();
 }
@@ -59,11 +54,6 @@ bool Npc::load()
 
 	reset();
 
-	if (!scriptInterface) {
-		scriptInterface = new NpcScriptInterface();
-		scriptInterface->loadNpcLib("data/npc/lib/npc.lua");
-	}
-
 	loaded = loadFromXml();
 	return loaded;
 }
@@ -80,8 +70,7 @@ void Npc::reset()
 	focusCreature = 0;
 	speechBubble = SPEECHBUBBLE_NONE;
 
-	delete npcEventHandler;
-	npcEventHandler = nullptr;
+	npcEventHandler.reset();
 
 	parameters.clear();
 	shopPlayerSet.clear();
@@ -208,12 +197,11 @@ bool Npc::loadFromXml()
 
 	pugi::xml_attribute scriptFile = npcNode.attribute("script");
 	if (scriptFile) {
-		npcEventHandler = new NpcEventsHandler(scriptFile.as_string(), this);
-		if (!npcEventHandler->isLoaded()) {
-			delete npcEventHandler;
-			npcEventHandler = nullptr;
+		auto handler = std::make_unique<NpcEventsHandler>(scriptFile.as_string(), this);
+		if (!handler->isLoaded()) {
 			return false;
 		}
+		npcEventHandler = std::move(handler);
 	}
 	return true;
 }
@@ -545,8 +533,6 @@ void Npc::closeAllShopWindows()
 	}
 }
 
-NpcScriptInterface* Npc::getScriptInterface() { return scriptInterface; }
-
 NpcScriptInterface::NpcScriptInterface() : LuaScriptInterface("Npc interface")
 {
 	libLoaded = false;
@@ -1087,8 +1073,14 @@ int NpcScriptInterface::luaNpcCloseShopWindow(lua_State* L)
 }
 
 NpcEventsHandler::NpcEventsHandler(const std::string& file, Npc* npc) :
-    npc(npc), scriptInterface(npc->getScriptInterface())
+    scriptInterface(std::make_unique<NpcScriptInterface>()), npc(npc)
 {
+	if (!scriptInterface->loadNpcLib("data/npc/lib/npc.lua")) {
+		std::cout << "[Warning - NpcLib::NpcLib] Can not load lib: " << file << std::endl;
+		std::cout << scriptInterface->getLastLuaError() << std::endl;
+		return;
+	}
+
 	loaded = scriptInterface->loadFile("data/npc/scripts/" + file, npc) == 0;
 	if (!loaded) {
 		std::cout << "[Warning - NpcScript::NpcScript] Can not load script: " << file << std::endl;
@@ -1119,7 +1111,7 @@ void NpcEventsHandler::onCreatureAppear(Creature* creature)
 	}
 
 	ScriptEnvironment* env = scriptInterface->getScriptEnv();
-	env->setScriptId(creatureAppearEvent, scriptInterface);
+	env->setScriptId(creatureAppearEvent, scriptInterface.get());
 	env->setNpc(npc);
 
 	lua_State* L = scriptInterface->getLuaState();
@@ -1142,7 +1134,7 @@ void NpcEventsHandler::onCreatureDisappear(Creature* creature)
 	}
 
 	ScriptEnvironment* env = scriptInterface->getScriptEnv();
-	env->setScriptId(creatureDisappearEvent, scriptInterface);
+	env->setScriptId(creatureDisappearEvent, scriptInterface.get());
 	env->setNpc(npc);
 
 	lua_State* L = scriptInterface->getLuaState();
@@ -1165,7 +1157,7 @@ void NpcEventsHandler::onCreatureMove(Creature* creature, const Position& oldPos
 	}
 
 	ScriptEnvironment* env = scriptInterface->getScriptEnv();
-	env->setScriptId(creatureMoveEvent, scriptInterface);
+	env->setScriptId(creatureMoveEvent, scriptInterface.get());
 	env->setNpc(npc);
 
 	lua_State* L = scriptInterface->getLuaState();
@@ -1190,7 +1182,7 @@ void NpcEventsHandler::onCreatureSay(Creature* creature, SpeakClasses type, cons
 	}
 
 	ScriptEnvironment* env = scriptInterface->getScriptEnv();
-	env->setScriptId(creatureSayEvent, scriptInterface);
+	env->setScriptId(creatureSayEvent, scriptInterface.get());
 	env->setNpc(npc);
 
 	lua_State* L = scriptInterface->getLuaState();
@@ -1216,7 +1208,7 @@ void NpcEventsHandler::onPlayerTrade(Player* player, int32_t callback, uint16_t
 	}
 
 	ScriptEnvironment* env = scriptInterface->getScriptEnv();
-	env->setScriptId(-1, scriptInterface);
+	env->setScriptId(-1, scriptInterface.get());
 	env->setNpc(npc);
 
 	lua_State* L = scriptInterface->getLuaState();
@@ -1244,7 +1236,7 @@ void NpcEventsHandler::onPlayerCloseChannel(Player* player)
 	}
 
 	ScriptEnvironment* env = scriptInterface->getScriptEnv();
-	env->setScriptId(playerCloseChannelEvent, scriptInterface);
+	env->setScriptId(playerCloseChannelEvent, scriptInterface.get());
 	env->setNpc(npc);
 
 	lua_State* L = scriptInterface->getLuaState();
@@ -1267,7 +1259,7 @@ void NpcEventsHandler::onPlayerEndTrade(Player* player)
 	}
 
 	ScriptEnvironment* env = scriptInterface->getScriptEnv();
-	env->setScriptId(playerEndTradeEvent, scriptInterface);
+	env->setScriptId(playerEndTradeEvent, scriptInterface.get());
 	env->setNpc(npc);
 
 	lua_State* L = scriptInterface->getLuaState();
@@ -1290,7 +1282,7 @@ void NpcEventsHandler::onThink()
 	}
 
 	ScriptEnvironment* env = scriptInterface->getScriptEnv();
-	env->setScriptId(thinkEvent, scriptInterface);
+	env->setScriptId(thinkEvent, scriptInterface.get());
 	env->setNpc(npc);
 
 	scriptInterface->pushFunction(thinkEvent);

diff --git a/src/npc.h b/src/npc.h
@@ -70,9 +70,10 @@ class NpcEventsHandler
 
 	bool isLoaded() const;
 
+	std::unique_ptr<NpcScriptInterface> scriptInterface;
+
 private:
 	Npc* npc;
-	NpcScriptInterface* scriptInterface;
 
 	int32_t creatureAppearEvent = -1;
 	int32_t creatureDisappearEvent = -1;
@@ -147,7 +148,7 @@ class Npc final : public Creature
 	void turnToCreature(Creature* creature);
 	void setCreatureFocus(Creature* creature);
 
-	NpcScriptInterface* getScriptInterface();
+	auto& getScriptInterface() { return npcEventHandler->scriptInterface; }
 
 	static uint32_t npcAutoID;
 
@@ -190,7 +191,7 @@ class Npc final : public Creature
 	std::string name;
 	std::string filename;
 
-	NpcEventsHandler* npcEventHandler;
+	std::unique_ptr<NpcEventsHandler> npcEventHandler;
 
 	Position masterPos;
 
@@ -207,8 +208,6 @@ class Npc final : public Creature
 	bool isIdle;
 	bool pushable;
 
-	static NpcScriptInterface* scriptInterface;
-
 	friend class Npcs;
 	friend class NpcScriptInterface;
 };