Jump to content
  • We need you!

    You must register to discover all the features of our community!

Some Problems With ShopEX(Memory Leak, open_shop)


Recommended Posts

I found such a problem while testing the npc.open_shop(vnum) function at shopex. I debugged and i found a memory leak.

https://puu.sh/FzIzk/d5f61845f5.mp4

 

Spoiler

Test: 1 normal shop, 2 shopex (9001,9002,9007)

m_map_pkShopByNPCVnum is ok, size is 3 (9001,9002,9007)

 

m_map_pkShop size is 2 normal shops adding with no problem,

but shopex objects adding only 1 time, example if you have 10 shopex only adding 1 object.

 

And this is destructor: (only m_map_pkShop is cleaning, so we have memory leak for other shopex objects) :

04a35f70af.png

shop_manager.cpp:

//Find
		if (pkShopEx->GetVnum() != 0 && m_map_pkShop.find(pkShopEx->GetVnum()) != m_map_pkShop.end())
		{
			sys_err("Shop vnum(%d) already exist.", pkShopEx->GetVnum());
			return false;
		}
		m_map_pkShop.insert(TShopMap::value_type(pkShopEx->GetVnum(), pkShopEx));

///Change
		if (m_map_pkShop.find(table.dwVnum) != m_map_pkShop.end())
		{
			sys_err("Shop vnum(%d) already exist.", table.dwVnum);
			return false;
		}
		m_map_pkShop.insert(TShopMap::value_type(table.dwVnum, pkShopEx));

//Find
void CShopManager::Destroy()
{
	TShopMap::iterator it = m_map_pkShop.begin();
	while (it != m_map_pkShop.end())
	{
		delete it->second;
		++it;
	}

	m_map_pkShop.clear();
}

///Change
void CShopManager::Destroy()
{
	for (auto it = m_map_pkShopByNPCVnum.begin(); it != m_map_pkShopByNPCVnum.end(); ++it)
		delete it->second;
	m_map_pkShopByNPCVnum.clear();
	m_map_pkShop.clear();
}

there is always a better method. This was the easiest solution that came to my mind.

Edited by Mali61 (see edit history)
  • Love 17

no paid service

use at least c++11 and VS19, otherwise I won't help.

Link to post
Posted (edited)
3 hours ago, Sherer said:

Just wrap it into smart pointer mate.
Or...
Clear data from m_map_pkShopByNPCVnum instead (since every, singleobject ends up there).

As I said there are other ways

 

Edited by Mali61
misunderstanding, he was just talking about the delete part (see edit history)

no paid service

use at least c++11 and VS19, otherwise I won't help.

Link to post

Well, the collector you came up with doesn't really does the job for me (pkShopEx is not stored anyway in m_map_pkShop since insert fails in that case).
That's why I mentioned it ;)

  • Love 1
Link to post
Posted (edited)
22 minutes ago, Sherer said:

Well, the collector you came up with doesn't really does the job for me (pkShopEx is not stored anyway in m_map_pkShop since insert fails in that case).
That's why I mentioned it ;)

are you talking about ReadShopTableEx function fail? If an error occurs, the server will not start anyway. 

db5f7fabee.png

but ok it's not problem for me, i edited topic for m_map_pkShopByNPCVnum

Edited by Mali61 (see edit history)

no paid service

use at least c++11 and VS19, otherwise I won't help.

Link to post
43 minutes ago, Mali61 said:

are you talking about ReadShopTableEx function fail? If an error occurs, the server will not start anyway. 

db5f7fabee.png

but ok it's not problem for me, i edited topic for m_map_pkShopByNPCVnum

I mean you'd suggested to change part of m_map_pkShop destruction (introduced collector etc.).

The problem is leak is not related to this container.

Have a look at piece of code you provided:
m_map_pkShop.insert
According to C++ docs insert method tries to add element if it not exists.
Because of that fact pkShopEx is never, ever added to map (if one exists, which you rightly mentioned).
Bear in mind that clearing m_map_pkShopByNPCVnum is not enough since it stores pointers to objects (allocated with new operators).
Your recent alterations already handle it well. Good job man.

for (auto it = m_map_pkShopByNPCVnum.begin(); it != m_map_pkShopByNPCVnum.end(); ++it)
		delete it->second;
Edited by Sherer (see edit history)
Link to post

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


  • Current Donation Goals

  • Activity

    1. 9

      [FIX] Great-Offshop MEMORY-LEAK

    2. 1

      acc system

    3. 9

      [FIX] Great-Offshop MEMORY-LEAK

    4. 3

      Where do i find Guild Chat in source?

    5. 3

      Where do i find Guild Chat in source?

    6. 0

      Extended DS Inventory

    7. 6

      Shadow devil tower.

    8. 9

      [FIX] Great-Offshop MEMORY-LEAK

    9. 22

      [FIX] Fixing Metin2 memory leaks

  • Recently Browsing

    No registered users viewing this page.

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