[fpc-devel] threads vs widestringmanager / crash

[Sven Barth]

Am 20.12.2022 um 15:08 schrieb Martin:
> Ok, I don't know too much about the whole initialization....
> But on the off chance of triggering some ideas, I throw in a couple of 
> my thoughts....
>
> On 19/12/2022 07:42, Sven Barth wrote:
>> Am 07.07.2018 um 15:04 schrieb Martin:
>>> So (guessing) the original issue may be due to the debugger. The 
>>> debugger interrupts the target early on. And that does create a 
>>> thread in the target.
>>> If such an external thread happens, would fcp execute the code in 
>>> question?
>>
>> This is likely to be the cause, cause the EXEC_TLS_CALLBACK is 
>> executed by Windows for every thread that is started for an 
>> application. And if the debugger triggers the start of a thread...
>
> Maybe, maybe not always? But, yes at least in the case that I 
> documented in 2018: 
> https://lists.freepascal.org/pipermail/fpc-devel/2018-July/039374.html
>
> Yet in the system unit:
>
> initialization
> .....
>   if not Assigned(CurrentTM.BeginThread) then
>   begin
>     InitHeap;
>     InitSystemThreads;
>   end;
>   SysInitExceptions;
>   initunicodestringmanager;
>   InitWin32Widestrings;
>
> So the WS-Mgr is only assigned after InitSystemThreads => which sets 
> up  RelocateThreadVar => which can call InitThread.
>
> So order may need to be changed there (if that is possible).
> Of course, the case where EXEC_TLS_CALLBACK calls InitThread remains.
>
> A lot of the WS-Mgr init is assigning pointers to the correct procedure.
> So if InitThread sees that this has not been done yet, InitThread 
> could call those.
> Even if 2 threads (main and the early thread) both assign values to 
> the pointer, they assign the same value, and so that should not do harm.
> Of course the thread init must not change the value, once it was set 
> by the main thread, in case usercode assigned a diff value (so maybe 
> in ThreadInit, use Interlocked....)
>
> This may mean to break "InitWin32Widestrings" into 2 parts, because it 
> also prepares some tables.
> But those are not needed for "GetStandardCodePageProc"
>
> So if the setting of the codepage related proc pointers are moved to a 
> separate init-method, then that can be called (if needed) by InitThread.
>
> --------------
> Or maybe if EXEC_TLS_CALLBACK calls InitThread, it can do the 
> (partial) init of WidestringMgr before? (if the order in the 
> initialization section can be changed to do the same?)
>
> --------------
> I am not sure if this code needs to worry about any of the other 
> WS-Mgr functions.
> Any thread that early is not started by Pascal code (neither user, nor 
> rtl). It will be a thread working some kernel proc. And that shouldn't 
> access the WS-Mgr?

Please stop focusing on the WideString manager. That is simply a 
symptom. The real cause is that a thread is created where there 
shouldn't be one and under normal circumstance there indeed will be none:
- if the initialization is executed as part of an application then there 
won't be any other thread, because the system initialization doesn't 
create one
- if the initialization is executed as part of a library then there 
won't be any other thread, because the initialization is run during the 
DLL_PROCESS_ATTACH call of the entry point which Windows executes with 
the global loader lock held, thus no thread can enter the entry point 
(which is also why there are some things you shouldn't do in a unit 
initialization)

The only cause for a thread to exist at that point is if another process 
calls CreateRemoteThread(Ex)() to create a thread in the process that is 
just initializing (Note: DebugBreakProcess() internally uses 
CreateRemoteThread(Ex)()). This means that the thread can interfere at 
*any* point of the initialization simply depending on the behavior of 
the Windows scheduler (as you hinted at in your other mail from today).

So the only logical solution is to stop the offending thread from 
executing or not to have it call InitThread() while the initialization 
section of the System unit is still running. The former might have 
unintended consequences (e.g. not being able to debug the unit 
initialization) while the later might work in most cases as long as that 
remote thread doesn't need to execute code that relies on InitThread().

Regards,
Sven