#18 NDC leaking on Win32

[emfau]

When checking for memory leaks on Win32 it turned out
that the NDC is leaking as already pointed by Colin
(https://sourceforge.net/tracker/index.php?func=detail&aid=645446&group_id=15190&atid=115190).
My understanding is that the leak is caused by the
destructor of ThreadLocalDataHolder.

inline ~Thread LocalDataHolder() { TlsFree(_key); };

It simply frees the TLS entry but without considering
to destruct the contained item. So the fix (at least to
my local problems) is as simple as

inline ~ThreadLocalDataHolder()
{
T *Item = (T *) TlsGetValue(_key);
if (Item)
delete Item;
TlsFree(_key);
};

This is only tested
with 0.3.4
in conjunction with a single project
If I'm missing somethings please let me know.

What happened to the solution Colin proposed for the
leaking problem of HierarchyMainter singleton and
_allAppender map?

Ahoi
Michael

[Bastiaan Bakker]

Logged In: YES
user_id=8050

Hi Micheal, your fix doesn't work: the ThreadLocalDataHolder
destructor will be called in only 1 thread (the main
thread). Therefore only the 'thread local object' held by
the current thread will be deleted, not the others.
The OmniThread library specfically has a omnithread:value_t
to derive from particularly to solve this problem. But since
the Win32 Threads API is C++ unaware, I suspect there isn't
a direct equivalent for MSThreads.hh.
Since this problem results in a 'static leak' of 1 object
per Thread (an not a growing leak), I'll let the issue rest
until log4cpp 0.4.x. There we'll try to tacle the other
leaks (like NDC, Category, etc.)
In the mean time you could work around the issue by using
boost threads instead.

[alexcohn]

Logged In: YES
user_id=1195458

I suggest a different patch for this problem:

use #include <list> and add the following to empty namespace
(line 29):

template<class T> class ownerlist : public std::list<T*>
{
public:
~ownerlist()
{
while (!empty())
{
T* pObj = front();
delete pObj;
pop_front();
};
};
};

ownerlist<NDC> _listNDC;

now, at line 83 (the number as after adding the piece
before), add the folowing:

_listNDC.push_back(nDC); // indicate that this pointer
should be released on shutdown

I agree that this leak is not a terrible one (unless
somebody generates many thousands of threads, in which case
this leak would be their least trouble). But cleaning it
helps much in the hunt for client program induced memory leaks.

Note that reset() should not be performed by
~ThreadLocalDataHolder(), otherwise same pointer will be
released twice.

Cheers,

Alex

[alexcohn]

Logged In: YES
user_id=1195458
Originator: NO

My fix revisited.

The fix I proposed on 2006-03-06 was not good: the list was getting longer and longer while new threads were created and exited. So, for the short-lived threads that exit gracefully, another implementation is suggested:

Enable the DllMain() by adding

#define LOG4CPP_SUPPLY_DLLMAIN

and add the handler for only one

case DLL_THREAD_DETACH:
{
NDC* pNDC = &NDC::getNDC();
delete pNDC;
break;
}

Tested for Win32, VS2005.