Jump to content

Fix Great Offshop Memory Leak


Volvox

Recommended Posts

  • Silver

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 by V0lvox
  • Lmao 1
  • Love 7
Link to comment
Share on other sites

  • Premium
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 by Lufbert
  • Love 2
Link to comment
Share on other sites

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

  • Contributor
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 by TokiSan
Link to comment
Share on other sites

  • Forum Moderator

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.


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

 

Edited by VegaS™
  • Metin2 Dev 27
  • Confused 1
  • Good 9
  • Love 1
  • Love 27
Link to comment
Share on other sites

  • Silver
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

  • Silver
5 hours ago, DeYaN. said:

I dont know how the source works , and i need to ask you :

  Hide contents

image.png

 

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 by Metin2 Dev
Core X - External 2 Internal
  • Love 1
Link to comment
Share on other sites

  • Developer
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 by Ikarus_

My youtube channel  on which you can see my works here

Link to comment
Share on other sites

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

  • Silver
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.


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

  • Developer
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 by Ikarus_

My youtube channel  on which you can see my works here

Link to comment
Share on other sites

  • 1 month later...


×
×
  • Create New...

Important Information

Terms of Use / Privacy Policy / Guidelines / We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.