[omniORB] omni_thread::init_t::~init_t will leak when FreeLibrary is called from a different thread as LoadLibrary

[Martin Trappel]

Hi all!

(omniORB 4.1.2 on Windows XP)

When loading the omnithread33_...dll dynamically with the Win32 function
LoadLibrary() and later releasing it with FreeLibrary()
... when the two calls are done from different threads the dtor
omni_thread::init_t::~init_t(void) can't do proper cleanup because
ThreadLocalStorage is used to store the pointers for the omni_thread
object. [see omnithread/nt.cc(455)]

What is actually the point of the
[omnithread.h(648)] static omni_thread::init_t omni_thread_init;
object? Why does it use TLS to remember some thread object?

thanks!
Martin


[CODE to repro. on VC8]

#include "stdafx.h"
#include <iostream>

LPCTSTR ominthread_module =
_T("C:\\Programme\\omniORB-4.1.2\\bin\\x86_win32\\omnithread33_vc8_rtd.dll");

HMODULE g_lib = NULL;

void load_fn(void* p) {
g_lib = LoadLibrary(ominthread_module);
}

bool terminated = false;
void free_fn(void* p) {
if(g_lib)
FreeLibrary(g_lib);
terminated = true;
}

int _tmain(int argc, _TCHAR* argv[])
{
//HMODULE lib = LoadLibrary(ominthread_module);
//if(lib)
// FreeLibrary(lib);

_beginthread(load_fn, 0, NULL);
Sleep(1000);
_beginthread(free_fn, 0, NULL);
Sleep(1000);
while(!terminated) {
Sleep(100);
}

return 0;
}

using namespace std;

class init_crt {
public:
init_crt() {
// Memory Leak Detection of CRT:
int nFlag = _CrtSetDbgFlag(_CRTDBG_REPORT_FLAG);
nFlag |= _CRTDBG_LEAK_CHECK_DF;
_CrtSetDbgFlag(nFlag);

::_CrtSetBreakAlloc(123);
}
};

static init_crt init_crt_obj;

[/CODE]

[Duncan Grisby]

Post by Martin Trappel
When loading the omnithread33_...dll dynamically with the Win32
function LoadLibrary() and later releasing it with FreeLibrary()
... when the two calls are done from different threads the dtor
omni_thread::init_t::~init_t(void) can't do proper cleanup because
ThreadLocalStorage is used to store the pointers for the omni_thread
object. [see omnithread/nt.cc(455)]
What is actually the point of the
[omnithread.h(648)] static omni_thread::init_t omni_thread_init;
object? Why does it use TLS to remember some thread object?

The per-thread state is used to implement omni_thread::self(). All
threads created by omnithread have the TLS set appropriately, but it is
also important that the main thread has a valid return from self(). The
expectation is that the omnithread library is imported by the
application's main thread, so the init_t object is used to set the
necessary TLS for the main thread.

That is, as you see, somewhat incompatible with dynamically loading the
omnithread library from one thread and unloading it from another. I'd
welcome a patch that implements a scheme to resolve that situation, but
I'm not certain it's possible.

Cheers,

Duncan.

[Martin Trappel]

Post by Duncan Grisby

Post by Martin Trappel
When loading the omnithread33_...dll dynamically with the Win32
function LoadLibrary() and later releasing it with FreeLibrary()
... when the two calls are done from different threads the dtor
omni_thread::init_t::~init_t(void) can't do proper cleanup because
ThreadLocalStorage is used to store the pointers for the omni_thread
object. [see omnithread/nt.cc(455)]
What is actually the point of the
[omnithread.h(648)] static omni_thread::init_t omni_thread_init;
object? Why does it use TLS to remember some thread object?

The per-thread state is used to implement omni_thread::self(). All
threads created by omnithread have the TLS set appropriately, but it is
also important that the main thread has a valid return from self(). The
expectation is that the omnithread library is imported by the
application's main thread, so the init_t object is used to set the
necessary TLS for the main thread.

On Windows, "statically" linked libraries are loaded by the main thread,
so there's no problem there. The problem is that we use omniORB from a
DLL-Project only that is dynamically loaded by our application.

Post by Duncan Grisby
That is, as you see, somewhat incompatible with dynamically loading the
omnithread library from one thread and unloading it from another. I'd
welcome a patch that implements a scheme to resolve that situation, but
I'm not certain it's possible.

I see. I'm not certain it's possible either, and I'm afraid due to the
small size of this CORBA project I'll not have time to investigate
further. :(

On question though:
Do you expect any problems from loading the omniThread DLL from a thread
that later goes away? (That would mean self() on the main thread will
fail?) I can live with the two(2) memory leaks due to the fact that the
unloading is not completed, but may there be other problems?
A workaround is of course to preload the omniThread DLL in our
main-thread, but I would rather avoid this.

cheers,
Martin

[Duncan Grisby]

Post by Martin Trappel
Do you expect any problems from loading the omniThread DLL from a
thread that later goes away? (That would mean self() on the main
thread will fail?) I can live with the two(2) memory leaks due to the
fact that the unloading is not completed, but may there be other
problems?

The thread that loads the omnithread dll is considered to be the main
thread. As long as you aren't using the main thread POA policy, it
doesn't matter if that thread goes away. I don't think there will be any
ill effects other than the small leak.

Cheers,

Duncan.