Jump to content

Fix Great Offshop Memory Leak


Recommended Posts

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
  • Love 6
  • Lmao 1
Link to post
  • VIP
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 post
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 post
  • VIP
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 post
  • 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).

Hidden Content

    Give reaction to this post to see the hidden content.

 

Edited by VegaS™
  • Love 13
  • Love 1
Link to post
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 post
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;

  • Love 1
Link to post
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 post

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 post
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 post
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 post
  • 1 month later...

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.



Shoutbox

Shoutbox

Chatroom Rules

 

Join our Discord

A request for help = Shoutbox Ban

Be respectful & Respect the rules

 

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