Jump to content

How To 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 (see edit history)
  • Love 6

spacer.png

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 (see edit history)
  • 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 (see edit history)
Link to post

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.

 

Edited by VegaS™ (see edit history)
  • Love 3
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". 

spacer.png

Link to post

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

Spoiler

image.png

 

i need to delete this all function , or only the "Msg" - from that line ? #same question for pkMsg;

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

spacer.png

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_ (see edit history)

My youtube channel  on which you can see my works here

Link to post
  • VIP
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 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.

 

 

spacer.png

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_ (see edit history)

My youtube channel  on which you can see my works here

Link to post
  • ASIKOO changed the title to How To Fix Great Offshop Memory Leak

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now


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