View Issue Details

IDProjectCategoryView StatusLast Update
0017300FPCRTLpublic2012-03-15 09:53
ReporterSven Barth Assigned ToMarco van de Voort  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformi386OSWin32 
Product Version2.5.1 
Summary0017300: Threads not created with BeginThread can't make use of FPC's RTL
DescriptionThis issue is Windows only, but might apply to other platforms as well.

If a thread, which is not created by BeginThread/TThread, executes Pascal code (common case: a thread created in a library calling a Pascal callback), this code can not make use of FPC's RTL (like exceptions and I/O), because its subsystems have to be initialized per thread.

Calling InitThread at the beginning of such Pascal code solves this issue.

A possible solution without calling InitThread manually:
The problem might be solved if a call to InitThread is placed inside SysRelocateThreadVar (inside the "if dataindex=nil" branch) in win/systhrd.inc, because the broken subsystems rely on threadvars and those are accessed by calling FPC_THREADVAR_RELOCATE (to which the value of SysRelocateThreadVar is assigned). Thus the thread should be initialised once such a "subsystem threadvar" is accessed.
But I haven't tested this and don't know if it would solve all subsystem problems.
Steps To ReproduceAttached is a example based on Alexander Grau's code on the Lazarus mailing list (link to thread see below).

There are two constants at the top of the code which influence the behavior:

USE_EXT_THREADS:
- false: Threads are created with a TThread class. The Execute method works as expected.
- true: Threads are created using a direct call to CreateThread. The "result" depends on the setting of USE_FPC_INIT

USE_FPC_INIT:
- false: all threads terminate uncatchable at the Writeln in ExternalThread. Wine debug output tells me that an unhandled exception has occured.
- true: same result as with USE_EXT_THREADS=false. All threads run as expected.
Additional InformationSee this thread on the Lazarus mailing list:
http://lists.lazarus.freepascal.org/pipermail/lazarus/2010-August/054727.html
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget
Attached Files

Activities

2010-08-29 19:33

 

externalthreads.pas (2,150 bytes)   
program externalthreads;

{$APPTYPE CONSOLE}
{$mode objfpc}{$H+}

uses
  Classes, Windows;

const
  // set this to false to enable the use of FPC's threads
  USE_EXT_THREADS = true;
  // set this to true to initialize FPC's RTL in external threads
  USE_FPC_INIT = false;
  COUNT = 30;

type
  TFPCThread = class(TThread)
  protected
    procedure Execute; override;
  end;

  TEmptyThread = class(TThread)
  protected
     procedure Execute; override;
   end;

procedure TFPCThread.Execute;
var
  t: tobject;
begin
  t := TObject.Create;
  try
    Writeln(t.ClassName); // <--- here we get *NO* crash
  finally
    t.Free;
  end;
end;

procedure TEmptyThread.Execute;
begin
  // do nothing
end;



function ExternalThread(param: Pointer): LongInt; stdcall;
var
  t: tobject;
begin
  if USE_FPC_INIT then
    // use some high number for the initial stack stack
    InitThread(1000000000);

  t := TObject.Create;
  try
    Writeln(t.ClassName); // <--- here we get a crash if USE_FPC_INIT is false
  finally
    t.Free;
  end;

  Result := 0;
end;


var
  ThreadID: DWord;
  i: integer;
  ThreadHandles: array[1..COUNT] of THandle;
  FPCthreads: array[1..COUNT] of tthread;

begin
  { initialise threading system by creating one internal thread }
   with TEmptyThread.Create(False) do
     try
       WaitFor;
     finally
       Free;
     end;

  WriteLn('Creating threads');
  for i := 1 to COUNT do
  begin
    if USE_EXT_THREADS then
    begin
      ThreadHandles[i] := CreateThread(nil, 0,
                            TFNThreadStartRoutine(@ExternalThread), Nil, 0,
                            ThreadID);
      if ThreadHandles[i] = 0 then
        Writeln('ERROR creating external thread');
    end else
      FPCThreads[i] := TFPCThread.Create(False);
  end;

  { let's sleep so that the following writeln appears after the writeln of
    the threads }
  Sleep(500);
  Writeln('Press a key to continue');
  Readln;

  WriteLn('Freeing threads');
  for i := 1 to COUNT do
  begin
    if USE_EXT_THREADS then
    begin
      if ThreadHandles[i] <> 0 then
        CloseHandle(ThreadHandles[i]);
    end else
      FPCThreads[i].Free;
  end;
end.
externalthreads.pas (2,150 bytes)   

Daniël Mantione

2010-08-29 19:44

administrator   ~0040609

I'm not sure if this should be considered a bug; if you create a thread outside the thread manager it is fair to leave it to the programmer to thread initialisation tasks the thread manager normally does.

SysRelocateThreadVar should contain as little code as possible for speed reasons, so putting the solution in there has its costs.

Sven Barth

2010-08-31 15:48

manager   ~0040679

Well... it seems that not even Delphi is supporting that automatically... There BeginThread is also recommended instead of CreateThread because of internal bookkeeping (just looking in forums, not the source of course).

Regarding my proposed solution: it would have only a cost the first time a threadvar is accessed in a thread, because the first time dataindex is Nil and afterwards dataindex is assigned a valid value. If InitThread would be placed in there it would only be called once.

But as this might not be considered as a bug (especially because Delphi doesn't support it either) this solution might not be necessary anymore at all ^^

Regards,
Sven

Jonas Maebe

2010-08-31 16:45

manager   ~0040686

Well, we added it for Unix platforms, so I'm not sure why we wouldn't add it for Windows if it's possible there as well.

Note that there are two aspects: intialisation (which can indeed be done the first time a threadvar is accessed), but also cleanup when the thread finishes! (and the solution there in cthreads is completely Unix-specific, so you'll have to find a Windows-specific mechanism for that as well)

Sven Barth

2010-08-31 17:17

manager   ~0040687

After crawling the MSDN library a bit I might have found a solution for the clean up (which is not as easy as on the Unix platforms). I'll try to cook something up in the next weeks. :)

Regards,
Sven

Anton Kavalenka

2010-09-17 21:23

reporter   ~0041173

And what if the thread was created in Delphi DLL and calls my FPC-created code,
can I use runtime in that callback?

Jonas Maebe

2010-09-17 21:31

manager   ~0041174

Not until this problem is fixed. There is no difference between a Delphi-created thread and a C++-created thread as far as the FPC run time library is concerned.

Sven Barth

2010-10-03 16:29

manager   ~0041443

Last edited: 2010-10-03 16:30

I've spent some time now to implement this and I must say: I don't know whether the use justifies the effort...

As Windows does not support callbacks for TLS creation and deletion I must basically do the following:
- create a worker thread which waits for any external thread handle (those are detected during SysRelocateThreadVar)
- once the thread handle becomes signaled: "impersonate" the terminated thread (by changing the dataindex inside SysRelocateThreadVar) and call DoneThread

Now this isn't as easy as it sounds:
- one call to WaitForMultipleObjects can only wait for $40 (=64) handles (and I must use one of those for an "update event"), so I have to implement something like "thread pooling" (because Windows 9x and Windows NT 4 don't support that) if we want to allow RTL usage for any number of external threads
- this thread pool system must be threadsafe => use of locks, hard to debug
- SysRelocateThreadVar is slowed done because of potential lock usage

So... should I continue my work on this or does someone have another/better idea?

Regards,
Sven

Jeppe Johansen

2010-10-03 16:47

developer   ~0041444

Is it possible to have the threadvar descriptor initialized to some specific area which is not NIL and mapped to the process? In that case we could make the SEH handler check whether a threadvar access has been tried by a thread that doesn't have it's threadvars initialized yet

For example add some extra code in syswin32_i386_exception_handler that checks the accessed address when a STATUS_ACCESS_VIOLATION occurs

Sven Barth

2010-10-03 17:19

manager   ~0041445

The problem isn't detecting uninitialized thread vars (that's easy), but finalizing them once the external thread has terminated. Because there's no possibilty to hook thread termination we need to wait until its handle is signaled. But then we are no longer in the context of that terminated thread so we must fake the location of the threadvars so that the RTL finalizes the correct values.

Regards,
Sven

José Mejuto

2010-10-03 18:04

reporter   ~0041447

Maybe it totally overkill but why not intercept the winapi CreateThread API function ? and maybe ExitThread.

Sven Barth

2010-10-03 18:53

manager   ~0041449

1. only because an application creates a thread using CreateThread doesn't mean that this thread calls a function in a library or a callback in the host binary (e.g. internal thread for polling a serial port)
2. CreateThread/ExitThread is used by FPC applications/libraries as well => we'd need to detect whether a library was compiled by FPC or not
3. the application might not statically import CreateThread but use LoadLibrary/GetProcAddress or it might even use NtCreateThread in ntdll.dll
4. what if two modules are doing that? (e.g. the FPC host binary and a FPC DLL)

In short: not only overkill, but also not even practical. But thanks for your interest :)

Regards,
Sven

José Mejuto

2010-10-03 23:45

reporter   ~0041465

1) Threads created in Window (I'm talking only about window) are being created using CreateThread WinAPI (with the exception of CreateRemoteThread).
2) No need to detect nothing the interception is at DLL level overwritting process thunk.
3) CreateThread is in user32.dll, so all windows applications are statically linked to it already.
4) I do not know the implications, but the interception is chained so at some point it should be possible to known if the job done in the interception is already done :-?

Anyway ok, maybe it is too complex to get the result.

Sven Barth

2010-10-04 10:10

manager   ~0041471

1. I don't want to detect external threads that never call FPC code
2. ???
3. If you import a DLL by using LoadLibrary/GetProcAddress you are bypassing the static import tables in the PE file (and thus the overwritten CreateThread entry). Also you can easily create a thread using NtCreateThread completly bypassing user32.dll (and thus the hook)
4. At load time it might be chained, but if a library is unloaded dynamically you break the chain irreparable

API hooks are always a bad idea. Especially as we only want to allow external threads to use FPC's RTL.

Regards,
Sven

Anton Kavalenka

2010-10-05 12:04

reporter   ~0041505

IMO marking thread in TLS after performing necessary per-thread RTL initialization is the most adequate approach.

Sven Barth

2010-10-05 14:16

manager   ~0041507

Would you please explain what you mean?

Regards,
Sven

Anton Kavalenka

2010-10-05 16:29

reporter   ~0041510

Last edited: 2010-10-05 16:32

@Sven
It is your proposition.
Allocating a slot in Thread-local storage that holds boolean (which indicates if we have done RTL initialization for this thread or not).
It seems enough. Maybe the boolean have to be surrounded with locations holding some magic numbers, which definitely describes - "yes, this thread is created by our BeginThread, so no initialization necessary.
Otherwise - no cookies, no flags - we are in foreign thread, so perform initialization.

Maybe just specific thread stack location holding magic cookie and flag would be enough.

Sven Barth

2010-10-05 17:14

manager   ~0041511

That's not the problem. The problem is to detect when the thread terminates so that we can finalize the RTL for this thread. And the only possibilty I'm aware of is to do a Wait* on the thread's handle (which we get during the initialization of the RTL for this thread).

Regards,
Sven

Anton Kavalenka

2010-10-05 18:03

reporter   ~0041515

Last edited: 2010-10-05 19:08

@Sven.

Maybe the information blocks about foreign threads should be collected, updated with thread handles and then periodically polled by garbage collector.

As far I know in Windows waiting on handle with timeout of 0 is nearly costless, so waiting of the thread still working returns WAIT_TIMEOUT. Waiting for terminated thread returns WAIT_OBJECT0 or error when the last handle to thread is closed.
POSIX pthread_timedjoin_np() behaves the same way.

Sven Barth

2010-10-05 20:41

manager   ~0041519

Hmm... that's also an idea... also simpler than the WaitForMultipleObjects one... but I wasn't sure about using Polling. I'll need to do some timings (and check the WaitForSingleObject code of ReactOS :P ).

Regards,
Sven

Michael Van Canneyt

2010-11-23 21:22

administrator   ~0043432

Small addition after a discussion on FPC core:
If I understand the MSDN page
 
http://msdn.microsoft.com/en-us/library/ms686997(v=VS.85).aspx
 
correctly, then intercepting DLL_THREAD_DETACH and DLL_PROCESS_DETACH should
take care of finalization for the case of a DLL. By adding some info to the TLS
block, we could see if the block was allocated internally or externally.

Sven Barth

2010-11-23 23:02

manager   ~0043439

Basically that's my idea as well.

I've already a collector thread running (tested on Win32 currently only), but it's not completly polished yet. After the thread works good enough, I'll test DLLs.

Regarding DLL_PROCESS_DETACH: What I see as a problem currently is that we remember the thread who called DLL_PROCESS_ATTACH and check whether this is the same thread in DLL_PROCESS_DETACH... if that's not the case though (which is perfectly legal) we might have the one or other memory leak if the process is not terminating, but just unloading the DLL...

Anton Kavalenka

2010-11-25 21:03

reporter   ~0043525

Wow: my other-language-compiled DLLs with threads start working from r16432
Thank you all!

Sven Barth

2010-11-25 22:41

manager   ~0043526

Oeehhh... What?! I haven't sent any patch yet, so what is different? O.o

Regards,
Sven

Anton Kavalenka

2010-11-26 09:44

reporter   ~0043537

r16432
michael
* Completely initialize threadvars when an external thread is detected. Needs still cleanup code (or a better thread initialization/finalization).

Michael Van Canneyt

2010-11-26 10:32

administrator   ~0043545

On an additional note:
Pierre Muller is working on TLS callbacks on windows.
Once they work they will be used instead to initialize/finalize threadvars.

Sven Barth

2010-11-26 14:22

manager   ~0043559

Is there any information / progress available on Pierre's work? Will it work on all three Windows targets?

Regards,
Sven

Sven Barth

2010-11-27 17:52

manager   ~0043615

Before I forget it:

>By adding some info to the TLS
block, we could see if the block was allocated internally or externally.

That's not necessary, because every DLL (and the host app) has its own TLS block (or better threadvar block) and thus the block in a FPC DLL will always be initialized by the DLL itself.

Regards,
Sven

Sven Barth

2011-04-10 13:17

manager   ~0047406

I have now published my implementation of a possibility to "garbage collect" the remains of external threads in "branches/svenbarth/collector".

The implementation is basically the following (I'm only talking about threads not created using BeginThread/TThread here):
* in a DLL:
- for threads, that are created after the library was loaded, the thread vars are initialized by DLLMain which is called with DLL_THREAD_ATTACH
- threads that were created before loading the library get their RTL initialized when they try to access a thread var (e.g. Output)
- both thread RTLs are freed when DLLMain is called using DLL_THREAD_DETACH (in case 2 the RTL is only freed if it was initialized of course)
* in an application:
- if a thread accesses a threadvar it's handle is duplicated and registered with the collector thread (which is now started if necessary)
- when a thread terminates it's handle becomes signaled and the RTL of the thread will be finalized (more about this below)

The collector works the following:
- the threads are registered using a linked list
- the collector waits using sleep and checks then whether one of the threads became signaled
- if one or more became signaled it aquires a threadvar lock (so that no other thread can access thread vars now) and sets a fake pointer to the TLS of the terminated thread so that SysRelocateThreadvar uses that pointer instead of the collector's pointer (this is the messy part of my solution)
- after that the list is cleaned

Overall it's not a nice solution, but it works and might even be necessary on older (non-NT) systems as they don't seem to support the TLS that Pierre is working on (at least a vanilla Windows 98 SE didn't support them).
My solution does not work on WinCE though as there a thread handle can't be duplicated and thus my whole registry approach does not work. What I've done (at the end) on WinCE though is copied the TLS initialization/finalization for DLLs as that wasn't done yet (this should be merged no matter what is done regarding the collector).

Regards,
Sven

Marco van de Voort

2012-01-21 12:58

manager   ~0055912

The last year there have been a lot of TLS related windows commits.

Status?

Anton Kavalenka

2012-02-10 07:56

reporter   ~0056708

Still not broken :)

Marco van de Voort

2012-02-17 10:28

manager   ~0056875

Last call before closing then? If sb thinks it should remain open, please comment, and update the title to more closely reflect the remaining problem

Sven Barth

2012-02-17 11:31

manager   ~0056878

What about Windows CE and Windows 9x? I haven't yet tested a new TLS-enabled Win32 binary on 9x systems and I don't know whether CE supports TLS callbacks.

Regards,
Sven

Marco van de Voort

2012-03-14 21:27

manager   ~0057655

Close. If there problems remain with certain platforms, please open specific tickets

Sven Barth

2012-03-15 09:53

manager   ~0057668

Ok, will do that if I encounter issues.

Regards,
Sven

Issue History

Date Modified Username Field Change
2010-08-29 19:33 Sven Barth New Issue
2010-08-29 19:33 Sven Barth File Added: externalthreads.pas
2010-08-29 19:44 Daniël Mantione Note Added: 0040609
2010-08-31 15:48 Sven Barth Note Added: 0040679
2010-08-31 16:45 Jonas Maebe Note Added: 0040686
2010-08-31 17:17 Sven Barth Note Added: 0040687
2010-09-17 21:23 Anton Kavalenka Note Added: 0041173
2010-09-17 21:31 Jonas Maebe Note Added: 0041174
2010-10-03 16:29 Sven Barth Note Added: 0041443
2010-10-03 16:30 Sven Barth Note Edited: 0041443
2010-10-03 16:47 Jeppe Johansen Note Added: 0041444
2010-10-03 17:19 Sven Barth Note Added: 0041445
2010-10-03 18:04 José Mejuto Note Added: 0041447
2010-10-03 18:53 Sven Barth Note Added: 0041449
2010-10-03 23:45 José Mejuto Note Added: 0041465
2010-10-04 10:10 Sven Barth Note Added: 0041471
2010-10-05 12:04 Anton Kavalenka Note Added: 0041505
2010-10-05 14:16 Sven Barth Note Added: 0041507
2010-10-05 16:29 Anton Kavalenka Note Added: 0041510
2010-10-05 16:32 Anton Kavalenka Note Edited: 0041510
2010-10-05 17:14 Sven Barth Note Added: 0041511
2010-10-05 18:03 Anton Kavalenka Note Added: 0041515
2010-10-05 19:08 Anton Kavalenka Note Edited: 0041515
2010-10-05 20:41 Sven Barth Note Added: 0041519
2010-11-23 21:22 Michael Van Canneyt Note Added: 0043432
2010-11-23 23:02 Sven Barth Note Added: 0043439
2010-11-25 21:03 Anton Kavalenka Note Added: 0043525
2010-11-25 22:41 Sven Barth Note Added: 0043526
2010-11-26 09:44 Anton Kavalenka Note Added: 0043537
2010-11-26 10:32 Michael Van Canneyt Note Added: 0043545
2010-11-26 14:22 Sven Barth Note Added: 0043559
2010-11-27 17:52 Sven Barth Note Added: 0043615
2011-04-10 13:17 Sven Barth Note Added: 0047406
2012-01-21 12:58 Marco van de Voort Note Added: 0055912
2012-01-21 12:58 Marco van de Voort Status new => feedback
2012-02-10 07:56 Anton Kavalenka Note Added: 0056708
2012-02-17 10:28 Marco van de Voort Note Added: 0056875
2012-02-17 11:31 Sven Barth Note Added: 0056878
2012-03-14 21:27 Marco van de Voort Status feedback => resolved
2012-03-14 21:27 Marco van de Voort Resolution open => fixed
2012-03-14 21:27 Marco van de Voort Assigned To => Marco van de Voort
2012-03-14 21:27 Marco van de Voort Note Added: 0057655
2012-03-15 09:53 Sven Barth Status resolved => closed
2012-03-15 09:53 Sven Barth Note Added: 0057668