Hello together!

As you might now, I'm working on a (according to Michael maybe temporary) solution for the clean up of external threads (see http://bugs.freepascal.org/view.php?id=17300 ).

My current state is that the collector thread which is used inside a "program" is working correctly and now I'm going to tackle the problem of DLLs.

I've looked at the necessary include files some time now and I might have spotted a potential problem with thread finalization in DLLs.

Consider the following: in a DLL we start a thread and once it has done its duty we call "EndThread" and return from the thread method (I'm thinking in terms of "BeginThread" not "TThread" for this example). After we've returned from the method Windows will call our DLL's entry point (and that of any other loaded DLL) with DLL_THREAD_DETACH where we call "DoneThread" because the current thread is not the "main" thread (another interesting story...).
The call of "DoneThread" is decorated with a comment "Assume everything is idempotent there". I'm hereby proposing that this assumption is not correct.

I've looked at "EndThread" and "DoneThread". "EndThread" calls the thread manager's "EndThread" method which on Windows calls "DoneThread"(!) and the API method "ExitThread".

"DoneThread" finalizes the RTL for that thread and starts with the WideString manager (not looked into that) and continues with the heap. Now the first thing the heap does is getting the address of its "freelists" record (which is a threadvar!) and plays around a bit with its contents (not really important for this discussion). "DoneThread" then continues finalizing other parts of the RTL and finally calls the thread managers "ReleaseThreadVars" method.

I sincerely hope that every alarm bell rings now. By calling "DoneThread" twice in a DLL we are basically accessing already freed memory: the thread var data.

As this is a very serious problem I now ask you how this should be solved:
1) keep it as it is (I hope not O.o)
2) fullfill the assumption and check the code which is called by "DoneThread" to be idempotent (which can break again at any time)
3) stop assuming that "DoneThread" is idempotent and do some checks here. E.g. by checking whether TlsGetValue(TLSKey) is Nil before calling "DoneThread" in DLL_THREAD_DETACH, which means that a) the RTL was never initialised (a thread that was created before DLL_PROCESS_ATTACH and never called into the DLL's code - see below) or b) the thread was already finalized.

Problem 3a is the following:
A thread is created in an application. Now the main thread loads a FPC library and some time after that the thread exits (it has never called any method in the DLL). Now the DLL receives a DLL_THREAD_DETACH and currently blindly finalizes a never initialized RTL (because it has never received a DLL_THREAD_ATTACH call for this thread!).

Modifying the RTL in this way might also solve the external thread problem for DLLs (I still have to think that one through, though).

Regards,
Sven

Pierre muller has a better solution.

He uses TLS callbacks. His solution is finished, but is in need of testing. If you want, I can send you his patch, and then you can test it too.

As for the DoneThread: your solution number 3 is the one to take. It is by far the safest. We'll need to implement it anyway like that to fix the case of the TLS done callback and the DLL_DETACH_THREAD.

Michael.

Yes, I'd like to test that. Especially as I'm curious how he's done that :D

As for the DoneThread: your solution number 3 is the one to take. It is by far the safest. We'll need to implement it anyway like that to fix the case of the TLS done callback and the DLL_DETACH_THREAD.

Great :D

Regards,
Sven

I sent it in private.

Michael.