Hello,

Some time ago I wrote my own thread implementation, and recently I went on the lookout for things I've missed in CThreads.pp in the source code of fpc 2.6.0 (FYI this is also the compiler I currently use). This way I found out about threadvar initialization and some other things I didn't know about. Very interesting!

Since my thread class already did everything that was written down in CThreads.pp, I started thinking `Hey, why not just write a thread manager, to get a full implementation?`. So I did this. In this process I discovered several bugs/inconsistencies in both my implementation and that of CThreads.

I'll start off with some strange things first:

    - In the source code of CThreads.pp there appears to be no field in
    the threadmanager record called `rtlEventSync`. Yet the compiler
    complains about the fact that this field is missing in my
    declaration of the TThreadManager record. This declaration looks as
    follows: `Const ZThreadManager = TThreadManager = (field: value;
    ........);`.
    - On Mac Os X 10.6.8 the compiler complains about the fact that my
    equivalent of the Function CBeginThread is incompatible with the one
    in the TThreadManager Record (The result types are incompatible,
    aswell as the type of ThreadId: Pointer [Linux; this is also what is
    typed in the source file]  TThreadId [Mac OS X]). This is very
    strange 'cause I copied the declaration of every function over from
    CThreads.pp. Should be the same, no?
    - Declaration of `pthread_key_create` differs from Mac OS X to
    linux? Very weird because the documentation for both OS'es says
    exactly the same, which is basically this:
        `Type TCProcedure = Procedure(arg: Pointer); cdecl;
        Function pthread_key_create(key: ppthread_key_t; DestructorFunc:
        TCProcedure): cint; cdecl; external name 'pthread_key_create';`

Then some things that seem unlogical to me:

- `Procedure IntbasiceventSetEvent` contains a Try...Finally block, while there is no code inbetween that could possibly raise an exception. Why is it there? (are some signals "converted" to an exception? If so, why isn't this in almost every other function as well?)

- Parameters `creationFlags` and `sa` in CBeginThread of CThreads.pp appear to be unused?

- `Procedure CInitCriticalSection` contains the remark `No recursive mutex support :/`; which is quite weird because a recursive mutex behaves *very* differently from a normal mutex. On some implementations this means that you will all of the sudden get deadlocks where you don't expect them. The fix for this would be to remove recursive mutex support for critical sections. A patch for this would be to replace the existing code with the one attached [CInitCriticalSection.pas].

- The types of TRTLCriticalSection and TThreadID are not defined in the same unit as the thread manager, which means that creating custom implementations of these (especially true for critical sections) is impossible. As an example: suppose I want to use an integer for busy waiting as a critical section, I would define TRTLCriticalSection as an integer. Now I am stuck with pthreads' mutex, which is not handy if you want to do this kinda thing with an integer.

And something I haven't yet investigated thoroughly:

- On Max OS X, `sem_open` appears not to be found. This might be a simply typo of mine, but maybe there's someone with the same problem?

Congratulations and thank you for reading this far ;-) If anyone could grant me some insights on one of these points it would be greatly appreciated.

Long days and pleasant nights to you all.

BTW: I only program unix OS'es nowadays, so don't shoot me if I missed some windows specific stuff.

Cthreads initialzes the thread manager record by explicitly assigning the thread manager record fields, not by declaring a typed constant. That means the compiler does not check for uninitialized fields. It seems this routine indeed still needs to be implemented for Unix platforms.

If you also use the same units in your interface as cthreads, yes.

What difference do you mean exactly?
Linux: pthread_key_create : Function(__key:ppthread_key_t;
__destr_function:__destr_function_t):longint;cdecl;
Mac OS X: function  pthread_key_create (p : ppthread_key_t;f:
__destr_func_t):cint; cdecl;external 'c';

It's a procvar under Linux because libpthread is loaded dynamically there. And while it definitely would be cleaner to use cint instead of longint in the Linux version, in practice the type is the same.

That was added by the person who did the very first implementation. There were in fact more try/finally's in the original implementation, but those have already been removed in the mean time. This one was apparently missed.

- Parameters `creationFlags` and `sa` in CBeginThread of CThreads.pp appear to be unused?

They are only used on Windows currently (they correspond to the corresponding arguments of the Windows CreateThread API). We should probably add a check for nil/0 and give an error if they are different on Unix.

Removing recursive mutex support would completely break Windows/Delphi/ past-FPC compatibility for critical sections, so that is definitely not going to happen. At best, we can give an error when creating the critical section if no recursive mutex support is available.

They are indeed defined according to the OS thread interface. If necessary, I guess we could change them all to pointers, although it would probably break a lot of code though (since currently, code can use variables of those types with native OS thread functions). It's not true that that the current type differences mean that a custom implementation is impossible though, although it will require some typecasting in your manager. You can also add a check like this to your code to ensure that you don't make wrong assumptions:

{$if sizeof(tthreadid) < sizeof(longint)}
{$error tthreadid must be at least the size of a longint for this
thread manager to function correctly}
{$endif}

Where is it not found/how are you looking for it? Or are you perhaps confusing it with sem_init, which indeed does not exist on Mac OS X?

Jonas

Once upon a time, on 11/16/2012 01:19 PM to be precise, Jonas Maebe said:

Right, I see. I've reported a bug on this matter.

Well, the compiler gives this error:
`Incompatible types: got
"<address of function(Pointer,QWord,TThreadFunc,Pointer,LongWord,var *Pointer*):^*untyped*;Register>" expected
"<procedure variable type of
function(Pointer,QWord,TThreadFunc,Pointer,LongWord,var *TThreadID*):^*TThreadRec*;Register>"
`

Rather weird, no? Have I missed something? Or is this due to the fact that I declare my threadmanager record using a typed constant?

Note: Th compiler also complains about several other functions/procedures involving the tpe TThreadID. Same error as the one above.

Well, I discovered that some code was contained in an $ifdef block, while it shouldn't. So there is indeed no difference between the two calls. Sorry for that.

I see. I've also reported a bug on this one.

Yep, as I said, I'm not into windows programming lately. So it basically has no meaning at all on non-windows OS'es?

Removing recursive mutex support would completely break Windows/Delphi/past-FPC compatibility for critical sections, so that is definitely not going to happen. At best, we can give an error when creating the critical section if no recursive mutex support is available.

Right, I didn't know this. The best solution would be, as you said, to give an error in case no recursive mutex support is available.

Indeed it would perhaps break to much code to do any good, especially true for TThreadID. Then again there are `eassier` types, for example TRTLCriticalSection, which never gets used as a parameter in a system call... Or am I wrong in saying that?

It's not true that that the current type differences mean that a custom implementation is impossible though, although it will require some typecasting in your manager.

Yep, that's what I did to give it a try, and of course it works, but it is, let's admit it, a rather hackish way and a waste of good memory. Also it doesn't really give you code that compiler everywhere. So in my opinion it can be called `Impossible`.

This is very interesting to generate `safer` code (i.e. only perform the typecast when there is enough memory), but it still doesn't give one a clean solution.

No, it's definetly sem_open, the litteral error is: `Error: Identifier not found "sem_open"`

My uses list contains the exact same units as in CThreads.pp, in the same order, so I don't see why the symbol isn't found. Also, the entire load of $idfdef's are also copied into my source code.

Undeffing `has_sem_open` makes the code compile.

Same goes for `SEM_FAILED`, but this is related to the very same issue I believe.

In what syntax mode are you compiling? Keep in mind that turning a procedure/function into a procvar is different in FPC/ObjFPC and TP/Delphi modes (with/out the @).

No, it's definetly sem_open, the litteral error is: `Error: Identifier not found "sem_open"` My uses list contains the exact same units as in CThreads.pp, in the same order, so I don't see why the symbol isn't found. Also, the entire load of $idfdef's are also copied into my source code.

The sem_open declaration is included in cthreads via rtl/darwin/pthread.inc (I know it's not actually a pthreads function, but that was the most logical place to put it)

Jonas

Once upon a time, on 11/16/2012 07:21 PM to be precise, Jonas Maebe said:

{$mode ObjFPC}, which is the same as cthreads.pp. Removal of the doesn't solve the problem. The thing that is weird about this is that the type I use for `TThreadID` is not self defined, it comes from one of the units used; which somehow is different from some other type that the TThreadManager record uses. Is there a way to figure out from where fpc gets a certain symbol via command line switch or something?

Copying the relevant code that is contained in `rtl/darwin/pthread.inc` over to my code seems to fix this issue, logically. I seem to have missed the `{$i pthread.inc}` line of the original CThreads.pp. Now, as you said, it isn't really a pthread function, but why not putting it in pthreads.pp? (thus copy the line {$i pthread.inc} over to pthreads.pp)

As I see it now, programmers don't have access to the symbols contained in the include file, unless they implement them themselves.

.
.

Not only that - it doesn't seem to be implemented for any of our platforms as far as I can see (at least neither Windows nor Unix nor OS/2). Our documentation says that it is obsolete and it should not be used. Considering this, we should probably remove it immediately because anyone trying to use it gets an immediate SIGSEGV due to uninitialized (nil) pointer (unless he uses his own thread manager, of course). If there are some reasons for keeping the parameter, we should at least modify the common RtlEventSync implementation in thread.inc to check for the nil value first and either do nothing or generate some more friendly error message.

Tomas

No. The easiest way to figure this one out is to open the source code in Lazarus, and use its "Find declaration" functionality (by default: position the cursor over an identifier and press alt/option-up).

The RTL's pthread.inc file only exists for internal RTL usage. Complete header translations under packages are more formally defined and generally should not start mixing in parts from different APIs. FPC indeed currently does not include a proper/full translation of the SYSV semaphore and shared memory APIs.

Jonas

Once upon a time, on 11/18/2012 11:43 AM to be precise, Jonas Maebe said:

Alright, I'll see what I can find, and I'll post it here when I've fixed it.

Once upon a time, on 11/18/2012 01:23 PM to be precise, Ewald said:

Right, I fixed this issue by adding a very simple type declaration at the top of the unit:

    Type
        TThreadID = System.TThreadID;

That I didn't think of this before...

It seems that the pthreads unit redefines the tthreadid type. That one should probably be removed.

Jonas

I've removed it. Moreover it was only done in the *bsd/darwin, haiku and beos versions of that unit.

Jonas

Once upon a time, on 11/19/2012 02:51 PM to be precise, Jonas Maebe said:

Cool!

That also explains why I didn't have the problem under linux...