PTHREAD_MUTEX_RECURSIVE

[Mo_Al_]

Hello

The Fl_Posix_System_Driver.cxx file has an 

```

#ifdef PTHREAD_MUTEX_RECURSIVE

```

to determine whether to use recursive locks when calling Fl::lock(). However this doesn't seem to work. I noticed that when running the code in helgrind and tsan, that lock_function_std() is called, none of the recursive variants are called. 

Testing with a simple cpp file:

```

#include <pthread.h>

#include <stdio.h>

int main() {

    #ifdef PTHREAD_MUTEX_RECURSIVE

    printf("Recursive\n");

    #else

    printf("Not\n");

    #endif

    if (PTHREAD_MUTEX_RECURSIVE) {

        printf("Recursive\n");

    } else {

        printf("Not\n");

    }

}

```

The pthread.h has PTHREAD_MUTEX_RECURSIVE as an enum value, so that wouldn't work in an ifdef. Not sure if older versions had it as a define.

[Greg Ercolano]

The pthread.h has PTHREAD_MUTEX_RECURSIVE as an enum value, so that wouldn't work in an ifdef. Not sure if older versions had it as a define.

    Good catch; this appears to be platform specific.

    On OSX it's a macro, on linux it's an enum.
    Might want to open an issue regarding this.

    --- /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include/pthread.h
    /*
     * Mutex type attributes
     */
    #define PTHREAD_MUTEX_NORMAL            0
    #define PTHREAD_MUTEX_ERRORCHECK        1
    #define PTHREAD_MUTEX_RECURSIVE         2
    #define PTHREAD_MUTEX_DEFAULT           PTHREAD_MUTEX_NORMAL


    --- /usr/include/pthread.h

    /* Mutex types.  */
    enum
    {
      PTHREAD_MUTEX_TIMED_NP,
      PTHREAD_MUTEX_RECURSIVE_NP,
      PTHREAD_MUTEX_ERRORCHECK_NP,
      [..]

[Bill Spitzak]

It does look like recursive lock emulation is done if it does not think this is defined.

I think it may have to add if's for various systems to know they have recursive locks, I think this is very common.

This gets into the weird "sys driver" stuff where compile-time decisions are implemented as function pointers. I really think that needs to be fixed. You can still put the implementations in different files, just put #if xxx around the include statements. The current api is certainly wasting a lot of time and is very hard to read or debug and you cannot put break points on the functions you think you are putting them on.

--
You received this message because you are subscribed to the Google Groups "fltk.general" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkgeneral...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/fltkgeneral/e044460a-00b8-bd38-9b37-71a7cbac3e71%40seriss.com.

[Albrecht Schlosser]

On 6/30/21 9:58 PM Bill Spitzak wrote:

It does look like recursive lock emulation is done if it does not think this is defined.

I think it may have to add if's for various systems to know they have recursive locks, I think this is very common.

This should obviously be done by a try_compile() or similar configure/CMake check then.

This gets into the weird "sys driver" stuff where compile-time decisions are implemented as function pointers. I really think that needs to be fixed. You can still put the implementations in different files, just put #if xxx around the include statements.

Been there, done that - and the decision was made to move to the driver stuff a long time ago. One of the driving persons was Matthias Melcher, you may want to contact him if you need more details on the pro's and con's. It seems he's not very often looking into this mailinglist these days.

The main point is to have a scalable approach, having to select between lots of different platforms (not only macOS, Unix/Linux and Windows but also the "pico", SDL, Android, and even more "platforms". There are also drivers that can be switched at runtime (printing devices for instance) which can't be done by simple "compile time decisions". But we said that already.

The current api is certainly wasting a lot of time and is very hard to read or debug and you cannot put break points on the functions you think you are putting them on.

I agree *only* to the point that this structure can be hard to read, but debugging is simple: put a breakpoint on the public function you want to debug and follow the code path. This will definitely lead you to the virtual method you need to debug.

On Wed, Jun 30, 2021 at 12:26 PM Greg Ercolano <er...@seriss.com> wrote:

The pthread.h has PTHREAD_MUTEX_RECURSIVE as an enum value, so that wouldn't work in an ifdef. Not sure if older versions had it as a define.

Bill, may I ask you politely not to top-post here? It's annoying and hard to read.

[Hannu Vuolasaho]

2021-06-30 22:26 UTC+03.00, Greg Ercolano <er...@seriss.com>:

>> The pthread.h has PTHREAD_MUTEX_RECURSIVE as an enum value, so that
>> wouldn't work in an ifdef. Not sure if older versions had it as a define.
>
>
> Good catch; this appears to be platform specific.

I just looked quickly on OpenBSD and it has both

enum pthread_mutextype {
PTHREAD_MUTEX_ERRORCHECK = 1, /* Error checking mutex */
PTHREAD_MUTEX_RECURSIVE = 2, /* Recursive mutex */
.....
#define PTHREAD_MUTEX_RECURSIVE PTHREAD_MUTEX_RECURSIVE

So there are many kind of implementations.

Best regards,
Hannu Vuolasaho

[Mo_Al_]

I looked at the pthread.h provided by mingw and it also has PTHREAD_MUTEX_RECURSIVE as a define. So the only (apparent) outlier seems to be Linux!

Maybe using something like

#if defined(PTHREAD_MUTEX_RECURSIVE) || defined(__USE_UNIX98)

Would cover all bases?

[Albrecht Schlosser]

On 7/1/21 11:55 AM Mo_Al_ wrote:

I looked at the pthread.h provided by mingw and it also has PTHREAD_MUTEX_RECURSIVE as a define. So the only (apparent) outlier seems to be Linux!

Maybe using something like

#if defined(PTHREAD_MUTEX_RECURSIVE) || defined(__USE_UNIX98)

Would cover all bases?

As I wrote before, a feature test using configure/CMake would IMHO be more appropriate, instead of "guessing" based on the platform. This is done using a short compile test with a source code that uses the symbol in question (PTHREAD_MUTEX_RECURSIVE). If the compilation succeeds, another well-defined macro (e.g. USE_MUTEX_RECURSIVE) can be defined in config.h. An example can be seen in CMake/posixScandir.cxx which tests if a particular scandir() prototype is defined in <dirent.h>.

Example test source code:

/*

FLTK feature test: do we have PTHREAD_MUTEX_RECURSIVE ?

Compile: gcc -c pthread_mutex_recursive.c

*/

#include <pthread.h>

int main() {

pthread_mutexattr_t attrib;

pthread_mutexattr_settype(&attrib, PTHREAD_MUTEX_RECURSIVE);

return 0;

}

If this compiles, then both <pthread.h> and PTHREAD_MUTEX_RECURSIVE are available.

[Albrecht Schlosser]

On 7/1/21 2:43 PM Albrecht Schlosser wrote:

Example test source code:

/*

FLTK feature test: do we have PTHREAD_MUTEX_RECURSIVE ?

Compile: gcc -c pthread_mutex_recursive.c

*/

#include <pthread.h>

int main() {

pthread_mutexattr_t attrib;

pthread_mutexattr_settype(&attrib, PTHREAD_MUTEX_RECURSIVE);

return 0;

}

If this compiles, then both <pthread.h> and PTHREAD_MUTEX_RECURSIVE are available.

I should have mentioned that this code compiles on Linux.

A slightly longer variant shows that PTHREAD_MUTEX_RECURSIVE is indeed not defined:

/*

FLTK feature test: do we have PTHREAD_MUTEX_RECURSIVE ?

Compile and run:

gcc -o main pthread_mutex_recursive.c -lpthread

./main; echo $?

*/

#include <pthread.h>

int main() {

pthread_mutexattr_t attrib;

pthread_mutexattr_settype(&attrib, PTHREAD_MUTEX_RECURSIVE);

#ifdef PTHREAD_MUTEX_RECURSIVE

return 1;

#else

return 2;

#endif

}

$ gcc -o main pthread_mutex_recursive.c -lpthread
$ ./main ; echo $?
2

Exit status is 2 (not defined).

[Daniel G.]

Can you try to build with -D_XOPEN_SOURCE=700 on the system that is causing troubles?

[Daniel G.]

Seems like the PTHREAD_MUTEX_RECURSIVE define is guarded in pthread.h and  -D_XOPEN_SOURCE=700 is the way to enable it.

[Manolo]

I've found these situations

- PTHREAD_MUTEX_RECURSIVE is #define'd :

macOS, NetBSD, pthreads-mingw64

- PTHREAD_MUTEX_RECURSIVE is an enum value :

Linux, FreeBSD, pthreads-mingw32

But PTHREAD_MUTEX_RECURSIVE is always usable as a C constant value.

We definitely have to remove the present usage of

#ifdef PTHREAD_MUTEX_RECURSIVE

I see two ways out :

1) Only use recursive mutexes and remove all the code devoted to the support of non-recursive mutexes.

That would be fine on all systems mentionned above. It would fail on systems where pthread.h exists but

PTHREAD_MUTEX_RECURSIVE is neither an enum nor a preprocessor variable. Do such systems exist?

2) define an FLTK-specific preprocessor variable that would be used instead of

#ifdef PTHREAD_MUTEX_RECURSIVE

and that would be always set in config.h to 1,  but that could be commented out to allow bulding of FLTK

on a hypothetical system lacking recursive mutexes.

Alternatively, the configure-time test Albrecht showed could put config.h in the adequate state.

Albrecht previously suggested to name such prepro variable USE_MUTEX_RECURSIVE

[Bill Spitzak]

It does look like assuming it works is acceptable. If a platform is found that it does not work on then this will not compile, and the substitute could be used with a #if to detect that platform.

I also think checking the return value of the setattr call is unnecessary (or it should be an assert).

This was originally stuck in there because on Windows (at that time) you could only make recursive mutexes and I was concerned about incompatibility. The way you are supposed to use the fltk lock, a non-recursive mutex would work as well.

It may be a good idea to have Windows use pthread.h as well, from what I have heard they are more efficient than the system ones.

--

You received this message because you are subscribed to the Google Groups "fltk.general" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkgeneral...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/fltkgeneral/0625f1d1-283b-4a75-ae40-dc8e04268587n%40googlegroups.com.

[Albrecht Schlosser]

As suggested, the OP (Mo_Al_ aka @MoAlyousef) opened a GitHub issue.
https://github.com/fltk/fltk/issues/245

We should move the discussion to this issue to avoid duplicate discussions.

Please add (copy) constructive proposals to this issue so they don't get lost. Thanks

On 7/2/21 6:21 PM Manolo wrote:

I've found these situations

- PTHREAD_MUTEX_RECURSIVE is #define'd :

macOS, NetBSD, pthreads-mingw64

- PTHREAD_MUTEX_RECURSIVE is an enum value :

Linux, FreeBSD, pthreads-mingw32

But PTHREAD_MUTEX_RECURSIVE is always usable as a C constant value.

We definitely have to remove the present usage of

#ifdef PTHREAD_MUTEX_RECURSIVE

I see two ways out :

1) Only use recursive mutexes and remove all the code devoted to the support of non-recursive mutexes.

That would be fine on all systems mentionned above. It would fail on systems where pthread.h exists but

PTHREAD_MUTEX_RECURSIVE is neither an enum nor a preprocessor variable. Do such systems exist?

-1

That's only guessing, guessing, guessing ... There are better alternatives (configure test).

2) define an FLTK-specific preprocessor variable that would be used instead of

#ifdef PTHREAD_MUTEX_RECURSIVE

and that would be always set in config.h to 1,  but that could be commented out to allow bulding of FLTK

on a hypothetical system lacking recursive mutexes.

Alternatively, the configure-time test Albrecht showed could put config.h in the adequate state.

+1

Albrecht previously suggested to name such prepro variable USE_MUTEX_RECURSIVE

FTR: I changed this to HAVE_PTHREAD_MUTEX_RECURSIVE in my comment to the GitHub issue:
https://github.com/fltk/fltk/issues/245#issuecomment-872231055

[Albrecht Schlosser]

On 7/2/21 1:12 AM 'Daniel G.' via fltk.general wrote:
> Can you try to build with -D_XOPEN_SOURCE=700 on the system that is
> causing troubles?

Thanks for your suggestion. I tried this:

$ cat pthread_mutex_recursive.c

/*
  FLTK feature test: do we have PTHREAD_MUTEX_RECURSIVE ?
  Compile and run:
    gcc -o main pthread_mutex_recursive.c -lpthread
    ./main; echo $?
*/
#include <pthread.h>
int main() {
  pthread_mutexattr_t attrib;
  pthread_mutexattr_settype(&attrib, PTHREAD_MUTEX_RECURSIVE);
#ifdef PTHREAD_MUTEX_RECURSIVE
  return 1;
#else
  return 2;
#endif
}

$ gcc -o main -D_XOPEN_SOURCE=700 pthread_mutex_recursive.c -lpthread &&
./main; echo $?
2

This still returns '2' which means PTHREAD_MUTEX_RECURSIVE is not de,
nofined.

Looking at pthread.h I can't see anything like '#define
PTHREAD_MUTEX_RECURSIVE', nor is there a reference to (_)XOPEN_SOURCE:

$ uname -sr
Linux 5.4.0-77-generic
$ egrep 'PTHREAD_MUTEX_RECURSIVE|XOPEN_SOURCE' /usr/include/pthread.h
  PTHREAD_MUTEX_RECURSIVE_NP,
  PTHREAD_MUTEX_RECURSIVE = PTHREAD_MUTEX_RECURSIVE_NP,
 { {  __PTHREAD_MUTEX_INITIALIZER (PTHREAD_MUTEX_RECURSIVE_NP) } }
   PTHREAD_MUTEX_RECURSIVE, PTHREAD_MUTEX_ERRORCHECK, or

I also checked that there's no other pthread.h on my system (except
those related to mingw cross tools).

[Ian MacArthur]

On 2 Jul 2021, at 19:27, Bill Spitzak wrote:
>
> It may be a good idea to have Windows use pthread.h as well, from what I have heard they are more efficient than the system ones.
>

Hmm, that hasn’t been my experience, but AFAIK there are a few different libs for implementing pthreads on Win32 so the behaviours may vary. I’m not saying that the Win32 locks are particularly efficient or low cost (they are not, in my tests), only that the pthread ports I’ve tested on Win32 are no better...

As a result, I always try to use the “native” locks, but YMMV...

[Albrecht Schlosser]

What about C++11 threads, performance-wise?

I suppose C++11 threads will use whatever the platform provides and thus
can maybe differ significantly, but maybe someone has some clues?

I know we're not using C++11 but we could probably do it as a build
option. Note that the Android port explicitly uses C++11 (or later
stuff) and we could probably do this as well on macOS and maybe
(optional) even on other platforms. Would this make any sense?

[Ian MacArthur]

On 6 Jul 2021, at 17:10, Albrecht Schlosser wrote:
>
> On 7/5/21 9:12 PM Ian MacArthur wrote:
>> On 2 Jul 2021, at 19:27, Bill Spitzak wrote:
>>> It may be a good idea to have Windows use pthread.h as well, from what I have heard they are more efficient than the system ones.
>> Hmm, that hasn’t been my experience, but AFAIK there are a few different libs for implementing pthreads on Win32 so the behaviours may vary. I’m not saying that the Win32 locks are particularly efficient or low cost (they are not, in my tests), only that the pthread ports I’ve tested on Win32 are no better...
>>
>> As a result, I always try to use the “native” locks, but YMMV...
>
> What about C++11 threads, performance-wise?

I do not know - I don’t really have a view on those, never having used (or measured!) them...
The code I have metrics for was originally an embedded model in vxWorks, we made some wrappers for the OS primitives so that we could build the same code on Win32 and Linux (it is *much* easier to test the models on the desktop rather than the embedded target...)
But obviously the timing is very different.
(In many ways... The vxWorks build is generally slower - lower power CPU etc. - but the scheduling primitives are much more consistent and generally quicker, for example.)
The vxWorks target we started with did not have C++11 support, so it has never been an option worth investigating.

>
> I suppose C++11 threads will use whatever the platform provides and thus can maybe differ significantly, but maybe someone has some clues?

I assume they wrap the system calls, but I do not know.

[Bill Spitzak]

You may be right that it is the C++ mutexes that are fairly efficient on Windows, not the pthreads.h ones.

The old school Window locks were all calls to functions in the Windows runtime library, and I think a lot of them instantly do a system trap. All modern systems compile into at least a few inline assembly instructions that make the most common route (an uncontested mutex, for instance) much faster, as they avoid the overhead of a function call.

--
You received this message because you are subscribed to the Google Groups "fltk.general" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkgeneral...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/fltkgeneral/60431152-20BB-4480-9AF8-A9734056C07A%40gmail.com.

[Ian MacArthur]

On Tuesday, 6 July 2021 at 21:37:21 UTC+1 Bill wrote:

You may be right that it is the C++ mutexes that are fairly efficient on Windows, not the pthreads.h ones.

The old school Window locks were all calls to functions in the Windows runtime library, and I think a lot of them instantly do a system trap.

To be honest, in my experience, that is not a behaviour that is unique to Windows - a lot of multi-tasking OS are inclined to treat any potential scheduling point as a good place to insert a trap and think about doing a switch.

I bang against this a fair bit when testing my embedded models on my host systems - a lot of the older models were multi-threaded but cooperatively scheduled, whereas "the same" code running on Linux or Win32 will be scheduled by the OS and that can have (frequently very!) surprising affects on the way the models behave...

 

All modern systems compile into at least a few inline assembly instructions that make the most common route (an uncontested mutex, for instance) much faster, as they avoid the overhead of a function call.

TBH, I tend to have gone the classic "embedded code" route and taken advantage of the fact that "all" the modern CPU's (even the very moderately spec'd ones I use) provide some sort of atomic primitives and compiler intrinsics to manipulate them. It can lead to a few ifdef's in the code (ARM vs x86 vs PPC etc.) but ducks the issue with whether the OS will reschedule on your mutex or not and avoids any function call overhead.