Active Member Volvox 1503 Posted October 23, 2020 Active Member Share Posted October 23, 2020 (edited) Here is my fix for a huge mem leak on great Offshop. If you use SQLMsg* pkMsg(DBManager::instance().DirectQuery(... : void CHARACTER::UpdateShopItems() At the End: delete Msg; ####################################################### void CHARACTER::LoadPrivShops() At the End: delete pkMsg; ####################################################### void CHARACTER::OpenShop(DWORD id, const char* name, bool onboot) At the End: pkMsg.reset(nullptr); ####################################################### ACMD(do_shop_update_item) After: if (CHARACTER_MANAGER::instance().GetCharactersByRaceNum(30000, i)) { CharacterVectorInteractor::iterator it = i.begin(); while (it != i.end()) { LPCHARACTER pc = *it++; if (pc) if (pc->GetRaceNum() == 30000 && pc->GetPrivShop() == shop_id) { pc->UpdateShopItems(); return; } } } Add: delete pkMsg; After: if (CHARACTER_MANAGER::instance().GetCharactersByRaceNum(30000, i)) { CharacterVectorInteractor::iterator it = i.begin(); while (it != i.end()) { LPCHARACTER pc = *it++; if (pc) if (pc->GetRaceNum() == 30000 && pc->GetPrivShop() == shop_id) { pc->UpdateShopItems(); return; } } } Add: delete pkMsg; After: TPacketShopUpdateItem packet; packet.shop_id = shop_id; db_clientdesc->DBPacket(HEADER_GD_SHOP_UPDATE_ITEM, 0, &packet, sizeof(packet)); } } Add: delete pkMsg; ####################################################### ACMD(do_shop_refresh_items) At the End: delete pkMsg; ####################################################### EVENTFUNC(fix_shop_event) Before: return PASSES_PER_SEC(SHOP_TIME_REFRESH * 60); Insert: delete pkMsg; Have a look at the names of Msg or pkMsg. These are just names, so change them, like your function need it !! Edited October 29, 2020 by V0lvox 1 7 Link to comment Share on other sites More sharing options...
JezMan 2 Posted October 23, 2020 Share Posted October 23, 2020 Thanks V0lvox Link to comment Share on other sites More sharing options...
Honorable Member Mali 41627 Posted October 23, 2020 Honorable Member Share Posted October 23, 2020 you don't have to do for smart ptr 4 Link to comment Share on other sites More sharing options...
AKUROS 27 Posted October 23, 2020 Share Posted October 23, 2020 5 hours ago, Mali61 said: you don't have to do for smart ptr why? Link to comment Share on other sites More sharing options...
Premium Lufbert 20 Posted October 23, 2020 Premium Share Posted October 23, 2020 (edited) 14 minutes ago, AKUROS said: why? Because as it names said it's "smart". When function execution ends pointer is automatically deallocated and ram is freed (of course only in this simple example with unique_ptr, normally that topic is more complicated for global variables, variables added to vectors etc.). Edited October 23, 2020 by Lufbert 2 Link to comment Share on other sites More sharing options...
AKUROS 27 Posted October 24, 2020 Share Posted October 24, 2020 21 minutes ago, Lufbert said: Because as it names said it's "smart". When function execution ends pointer is automatically deallocated and ram is freed (of course only in this simple example with unique_ptr, normally that topic is more complicated for global variables, variables added to vectors etc.). That's right! Thanks Link to comment Share on other sites More sharing options...
DeYaN. 29 Posted October 27, 2020 Share Posted October 27, 2020 Hi there, without thix fix what happen ? Link to comment Share on other sites More sharing options...
Active Member Volvox 1503 Posted October 28, 2020 Author Active Member Share Posted October 28, 2020 Memory overflow, because he always allocate mem and never delete the allocation Link to comment Share on other sites More sharing options...
Contributor Toki.San 1962 Posted October 28, 2020 Contributor Share Posted October 28, 2020 (edited) On 10/23/2020 at 1:35 PM, V0lvox said: ####################################################### void CHARACTER::OpenShop(DWORD id, const char* name, bool onboot) At the End: pkMsg.reset(nullptr); ####################################################### I try This and have this error: compile char.cpp char.cpp: In member function 'void CHARACTER::OpenShop(DWORD, const char*, bool)': char.cpp:8086: error: request for member 'reset' in 'pkMsg', which is of non-class type 'SQLMsg*' char.cpp:8086: error: 'nullptr' was not declared in this scope Edited October 28, 2020 by TokiSan Link to comment Share on other sites More sharing options...
Forum Moderator VegaS™ 10239 Posted October 28, 2020 Forum Moderator Share Posted October 28, 2020 (edited) Just if you use SQLMsg* pkMsg then it's called a raw pointer and you've to delete it manually, but for std::unique_ptr<SQLMsg>, you don't have to do it, has no effects the reset method in this case. https://docs.microsoft.com/en-us/cpp/cpp/raw-pointers?view=vs-2019 https://docs.microsoft.com/en-us/cpp/cpp/smart-pointers-modern-cpp?view=vs-2019 https://en.cppreference.com/w/cpp/memory/unique_ptr The benefit of using smart pointers like unique_prt || shared_ptr is that they automatically delete the pointed object when no longer needed (i.e. out of scope). This is the hidden content, please Sign In or Sign Up Edited December 3, 2020 by VegaS™ 28 1 9 1 28 Check my GitHub Profile Click to find all the threads started by me [TOOL] Text file loader + JSON Link to comment Share on other sites More sharing options...
Active Member Volvox 1503 Posted October 29, 2020 Author Active Member Share Posted October 29, 2020 16 hours ago, TokiSan said: I try This and have this error: compile char.cpp char.cpp: In member function 'void CHARACTER::OpenShop(DWORD, const char*, bool)': char.cpp:8086: error: request for member 'reset' in 'pkMsg', which is of non-class type 'SQLMsg*' char.cpp:8086: error: 'nullptr' was not declared in this scope Just use your brain and have a look, what name your variable has. Mine is "pkMsg", maybe yours is "Msg". Link to comment Share on other sites More sharing options...
DeYaN. 29 Posted October 30, 2020 Share Posted October 30, 2020 (edited) I dont know how the source works , and i need to ask you : Spoiler i need to delete this all function , or only the "Msg" - from that line ? #same question for pkMsg; Edited August 25, 2022 by Metin2 Dev Core X - External 2 Internal Link to comment Share on other sites More sharing options...
Active Member Volvox 1503 Posted October 30, 2020 Author Active Member Share Posted October 30, 2020 (edited) 5 hours ago, DeYaN. said: I dont know how the source works , and i need to ask you : Hide contents i need to delete this all function , or only the "Msg" - from that line ? #same question for pkMsg; You dont need to delete anything, Just write at the end of this function: delete Msg; Edited August 25, 2022 by Metin2 Dev Core X - External 2 Internal 1 Link to comment Share on other sites More sharing options...
Goof 2 Posted November 6, 2020 Share Posted November 6, 2020 (edited) https://metin2.download/picture/011W569kW3jOYnX4d6d34qVPKg78jce6/.png mine is pkMsg look https://metin2.download/picture/68nnI330Fjl67N7V9yo5I8FjJh88AHS9/.png Edited September 4, 2022 by Metin2 Dev Core X - External 2 Internal Link to comment Share on other sites More sharing options...
Developer Ikarus_ 2413 Posted November 7, 2020 Developer Share Posted November 7, 2020 (edited) On 10/28/2020 at 4:55 PM, TokiSan said: I try This and have this error: compile char.cpp char.cpp: In member function 'void CHARACTER::OpenShop(DWORD, const char*, bool)': char.cpp:8086: error: request for member 'reset' in 'pkMsg', which is of non-class type 'SQLMsg*' char.cpp:8086: error: 'nullptr' was not declared in this scope nullptr is defined since c++11 and i guess you are using reset on a raw pointer (the reset wasn't really needed btw) Edited November 7, 2020 by Ikarus_ My youtube channel on which you can see my works here Link to comment Share on other sites More sharing options...
Contributor Toki.San 1962 Posted November 7, 2020 Contributor Share Posted November 7, 2020 8 hours ago, Ikarus_ said: nullptr is defined since c++11 and i guess you are using reset on a raw pointer (the reset wasn't really needed btw) Umh nope, the problem was msg Instead of pkmsg. Link to comment Share on other sites More sharing options...
developerares 1 Posted November 7, 2020 Share Posted November 7, 2020 Hi, Quote std::unique_ptr<SQLMsg> Msg(DBManager::instance().DirectQuery("SELECT id,vnum,count,display_pos,price,price_cheque,transmutation,%s,%s from player_shop_items where shop_id='%d'", szSockets, szAttrs, GetPrivShop())); SQLResult * Res = Msg->Get(); if (Res->uiNumRows > 0) Is there a problem with this code? I have smart ptr. @V0lvox Thank you. Link to comment Share on other sites More sharing options...
Active Member Volvox 1503 Posted November 7, 2020 Author Active Member Share Posted November 7, 2020 6 hours ago, developerares said: Hi, Is there a problem with this code? I have smart ptr. @V0lvox Thank you. On 10/28/2020 at 5:20 PM, VegaS™ said: Just if you use SQLMsg* pkMsg then it's called a raw pointer and you've to delete it manually, but for std::unique_ptr<SQLMsg>, you don't have to do it, has no effects the reset method in this case. https://docs.microsoft.com/en-us/cpp/cpp/raw-pointers?view=vs-2019 https://docs.microsoft.com/en-us/cpp/cpp/smart-pointers-modern-cpp?view=vs-2019 https://en.cppreference.com/w/cpp/memory/unique_ptr The benefit of using smart pointers like unique_prt || shared_ptr is that they automatically delete the pointed object when no longer needed (i.e. out of scope). void UseRawPointer() { // Using a raw pointer -- not recommended. SQLMsg * pkMsg(DBManager::instance().DirectQuery("SELECT * FROM player.player")); // Use pkMsg... uint32_t uiNumRows = pkMsg->Get()->uiNumRows; // Don't forget to delete! delete pkMsg; } void UseSmartPointer() { // Declare a smart pointer on stack and pass it the raw pointer. std::unique_ptr<SQLMsg> pkMsg(DBManager::instance().DirectQuery("SELECT * FROM player.player")); // Use pkMsg... auto uiNumRows = pkMsg->Get()->uiNumRows; //... } // pkMsg is deleted automatically here. Link to comment Share on other sites More sharing options...
Developer Ikarus_ 2413 Posted November 8, 2020 Developer Share Posted November 8, 2020 (edited) On 10/28/2020 at 4:55 PM, TokiSan said: char.cpp:8086: error: request for member 'reset' in 'pkMsg', which is of non-class type 'SQLMsg*' char.cpp:8086: error: 'nullptr' was not declared in this scope Uhm, yep, nullptr is not declared means you are not using c++11(+) and pkMsg is of non-class type SQLMsg* means you are using .reset on a raw pointer (and .reset is not needed even for smart ptr). You solved differently but maybe someone is interested in knowing why errors are received when they are received, as well as knowing how to fix it. On 11/7/2020 at 2:21 PM, developerares said: Is there a problem with this code? I have smart ptr. No it is correct. std::unique_ptr<SQLMsg> will delete the memory from heap when its destructor is called. smart pointers have the overload of the operator -> and this help to use them as classic (raw) pointers. (e.g. where you used Msg->Get()) Edited November 8, 2020 by Ikarus_ My youtube channel on which you can see my works here Link to comment Share on other sites More sharing options...
developerares 1 Posted November 9, 2020 Share Posted November 9, 2020 Thank you @V0lvox and @Ikarus_ 1 Link to comment Share on other sites More sharing options...
Flourine 105 Posted December 19, 2020 Share Posted December 19, 2020 Just use smart pointers Link to comment Share on other sites More sharing options...
Recommended Posts