ASTERISK-01396: FreeBSD 4.9 and MacOS X lack recursive lock initializers

[Home]

Summary: ASTERISK-01396: FreeBSD 4.9 and MacOS X lack recursive lock initializers

Reporter: Olle Johansson (oej) Labels:

Date Opened: 2004-04-12 05:31:15 Date Closed: 2008-01-15 14:57:43.000-0600

Priority: Minor Regression? No

Status: Closed/Complete Components: Core/General

Versions: Frequency of
Occurrence

Related
Issues:

Environment: Attachments: ( 0) freebsd_mutex.txt
( 1) lock-h.txt
( 2) lock-h-ifdef.dif
( 3) mutex.dif
( 4) mutex3.dif
( 5) mutex4.dif
( 6) mutex5.dif
( 7) mutextest.c
( 8) OpenBSD.diff
( 9) openbsd-mutex-patch.txt

Description: I can't compile Asterisk cvs head since the addition of recursive thread mutex stuff...

Does this require a special version of pthreads or gcc?

****** ADDITIONAL INFORMATION ******

Quoting Mark on the mailing list:
"Recent, requisite changes to Asterisk (most notably the RECURSIVE thread
transition and use of gethostbyname_r) have broken compatibility with BSD
which does not seem to have direct equivalents for some of these Linux
calls. Are there any BSD coders still out there who will be able to fix
these for BSD or are we giving up on BSD compatibility?"

Comments: By: Mark Spencer (markster) 2004-04-12 09:37:27

Any FreeBSD people out there that can comment? Recursive mutexes are here to stay -- we just can't keep maintaining all the workarounds to be able to support the "fast" style.
By: wiking (wiking) 2004-04-12 12:00:35

well i've got the same error on my OpenBSD 3.4, with the development release of asterisk.

as from http://www.openbsd.org/cgi-bin/man.cgi/usr/include/pthread.h
you can see that there are:
enum pthread_mutextype {
PTHREAD_MUTEX_ERRORCHECK = 1, /* Default POSIX mutex */
PTHREAD_MUTEX_RECURSIVE = 2, /* Recursive mutex */
PTHREAD_MUTEX_NORMAL = 3, /* No error checking */
MUTEX_TYPE_MAX
};

where PTHREAD_MUTEX_RECURSIVE would stand for PTHREAD_MUTEX_RECURSIVE_NP as i can see.
By: Olle Johansson (oej) 2004-04-12 14:48:05

#define AST_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
#define AST_MUTEX_KIND PTHREAD_MUTEX_RECURSIVE

in lock.h makes it compile and work without crashes.
The _NP stuff is 'non posix'
By: wiking (wiking) 2004-04-12 15:54:45

works fine with OpenBSD too :P
By: James Golovich (jamesgolovich) 2004-04-12 15:58:37

Fixed in CVS. Not with the exact patch, but something that should be a bit more portable
By: Mark Spencer (markster) 2004-04-12 16:29:55

I'm not sure that's valid. After all, this means that pthread mutexes which are called with ast_mutex_t foo = AST_MUTEX_INITIALIZER will not support recursion. I don't know of any off the top of my head that that would apply to, *but* still it could cause problems in BSD compiles that would not occur in the Linux ones.
By: wiking (wiking) 2004-04-12 17:08:32

well the mutex won't be initialized as recursive, but it's "kind" (AST_MUTEX_KIND) will be recursive, so it'll be a recursive mutex (of course if AST_MUTEX_KIND is ever used...)

while i was debugging with oej, i've looked around in linux pthread.h, as i can see there's nothing 'special' in PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP, so the mutex you init with simple PTHREAD_MUTEX_INITIALIZER will be able to 'work' as a recursive mutex (too)
By: Mark Spencer (markster) 2004-04-12 17:56:26

Okay, correction, there *are* places recursiveness is required. My asterisk wouldn't even start up, not even on linux, because you can't check for _NP to be defined since it's part of an enum, not a #define. I had to revert the patch entirely. It sounds like we're going to have to have a check against BSD not Linux.
By: Mark Spencer (markster) 2004-04-12 18:00:03

And yes, to clarify, the AST_MUTEX_KIND is only used when ast_mutex_init is used and is *not* used when they're declared statically and set to AST_MUTEX_INITIALIZER.
By: Olle Johansson (oej) 2004-04-13 13:36:45

So we need an AST_MUTEX_INITIALIZER that activates recursive mutexes on *BSD?
By: Mark Spencer (markster) 2004-04-14 14:52:18

Yes, or alternatively we have to get rid of all places where AST_MUTEX_INITIALIZER is used. Is it part of the POSIX spec that you can have an initializer that is setup for recursive mutexes?
By: Olle Johansson (oej) 2004-04-20 11:11:03

Added openbsd patch from ASTERISK-1461 - please test
By: Olle Johansson (oej) 2004-04-22 04:53:03

Maybe this can give a mutex-aware programmer some hints:

http://kerneltrap.org/node/view/2395
By: Olle Johansson (oej) 2004-04-22 04:56:10

Postgres seems to have gone through all of this
http://archives.postgresql.org/pgsql-patches/2003-09/msg00222.php
...maybe we should look there for advice?
By: Mark Spencer (markster) 2004-04-22 12:16:10

I'm merging in another bug related to not being able to get gethostbyname_r on FreeBSD.
By: Olle Johansson (oej) 2004-04-24 16:17:22

Added an ugly patch for gethostbyname, please test if this works for you. Seems to work in my installation.

I'm a bit over my head doing this patch, so please correct me, assist me, to solve this problem.
By: Mark Spencer (markster) 2004-04-26 08:59:34

You could wrap your routine in a mutex, but it still wouldn't work because the hostent struct has pointers into static data that is not the hp entry itself (that's why gethostbyname_r takes so many arguments).
By: Roy Sigurd Karlsbakk (rkarlsba) 2004-04-27 13:26:48

so what is there to do if I want to run * on osx? wait for the apple guys to fix it?
By: Olle Johansson (oej) 2004-04-27 15:00:02

This dirty little patch to asterisk.c seems to get my Asterisk running, but propably at high risk. Haven't crashed yet.
By: adam (adam) 2004-04-28 02:41:03

Regarding the gethostbyname_r workaround for BSD, prehaps wrap a mutex around it? Otherwise, it's a tad dodge
By: Olle Johansson (oej) 2004-04-28 02:50:14

Please check the postgresql URL above. Mark checked it and thought it was the right way to go. Now, we need coders.
By: adam (adam) 2004-04-28 03:03:39

From my understanding of that patch (I hate reading patches), my suggestion is still valid. I think just add locking for freebsd's gethostbyname and problem solved (we can add support for the other OS's when needed)
By: mwild (mwild) 2004-04-28 03:50:55

I've added a gethostbyname_r from

http://www.cygwin.com/ml/cygwin/2004-04/msg00532.html

which works just fine for me under FreeBSD-current.
By: Olle Johansson (oej) 2004-04-28 04:36:18

Added patch for asterisk.c. Works for me as well on 4.9

Test it and confirm here, please.

mwild: THANK YOU!!!!

Now, let's try to nail down recursive mutexes.
By: Olle Johansson (oej) 2004-04-28 10:55:15

Patch for gethostbyname_r added to CVS. Thank you, Mark!!!

Now: Recursive mutexes. Anyone?
By: mwild (mwild) 2004-04-28 14:35:24

About the initializers: there could be a problem here, because a
pthread_mutex_t in *BSD (at least FreeBSD) is a pointer, whereas in Linux
it seems to be a struct. So, whereas in FreeBSD I have:

#define PTHREAD_MUTEX_INITIALIZER NULL
#define PTHREAD_COND_INITIALIZER NULL
#define PTHREAD_RWLOCK_INITIALIZER NULL

In the Linux header file I found (might be old..) this looks like:

#define PTHREAD_MUTEX_INITIALIZER \
{0, 0, 0, PTHREAD_MUTEX_TIMED_NP, __LOCK_INITIALIZER}
#ifdef __USE_GNU
# define PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP \
{0, 0, 0, PTHREAD_MUTEX_RECURSIVE_NP, __LOCK_INITIALIZER}

The way I see this, a pthread_mutex_t can only be initialized to a
"defined not set" value, but not to specific attributes. I don't
know whether either of these implementations is violating POSIX
by providing for (or not providing for) static attribute initialization.

If the code can't be cleaned up otherwise, and since Asterisk is _only_
using recursive mutexes, we could just wedge (lock.h) into
ast_mutex_lock() and ast_mutex_trylock() and do something along:

#ifdef FreeBSD
if (!t->mutex) {
ast_mutex_init(t);
}
#endif

This would require FreeBSD to always using the wrapper functions in
lock.h.
By: Olle Johansson (oej) 2004-04-28 14:41:07

There's a port named "linuxthreads". COuld you look into that, to see if that's a way forward with this problem?
By: mwild (mwild) 2004-04-28 14:53:38

linuxthreads is a back port of linux threads. Going that way would
mean to loose many of the recent improvements to kernel threads, and
it would also introduce an external dependency into this project. I
think the above suggested approach is much cleaner than completely
throwing out the native thread library.
By: mwild (mwild) 2004-04-28 15:05:16

Please move the gethostbyname_r() function into a dedicated
source file. Within asterisk.c some of its internal code is
colliding with asterisk defines() from lock.h:

asterisk.c: In function `gethostbyname_r':
asterisk.c:1707: error: syntax error before "__mutex"
asterisk.c:1708: warning: implicit declaration of function `use_ast_mutex_lock_instead_of_pthread_mutex_lock'

since it's replacing a not-existing system function, it shouldn't
really be within application scope.
By: Olle Johansson (oej) 2004-04-28 15:09:46

Ok, so the linuxthreads was not a way forward. I can't judge your proposal, it's way over my head... Anyone else that have any comments?
By: Mark Spencer (markster) 2004-04-28 15:48:47

Okay I fixed the gethostbyname_r to use the ast_ mutex functions. still need a solution to the rest.
By: semhoun (semhoun) 2004-04-28 16:12:39

I add a new patch for OpenBSD to compile with the function gethostbyname_r
By: Roy Sigurd Karlsbakk (rkarlsba) 2004-04-29 05:00:20

If the final goal is to achive portability, is it really a good idea to require a linuxthreads port to the platform?
By: Roy Sigurd Karlsbakk (rkarlsba) 2004-05-01 03:01:10

PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP isn't defined anywhere in OS X. I can't find any _RECURSIVE entries at all...
By: Mark Spencer (markster) 2004-05-01 17:21:56

Without recursive mutexes you're surely doomed. What's the deal with OSX anyway? I had to find emulation for dlopen and poll, and now this? I thought it was unix ;-)
By: Brian West (bkw918) 2004-05-01 18:20:07

HAHAHAHA good one :) They sure smack that unix logo on it.
By: Roy Sigurd Karlsbakk (rkarlsba) 2004-05-01 19:07:58

So should we add a

#if defined (__APPLE__)
# error "fuck you"
#endif

...or could this be fixed?
By: Mark Spencer (markster) 2004-05-02 00:04:24

I don't know if it can be fixed. Care to research it?
By: John Todd (jtodd) 2004-05-03 22:26:48

OK, I'm willing to test with FreeBSD 5.2.1 and OpenBSD 3.4 but I'm uncertain given the list of patches/etc. above which I'm to apply in order to test. Can someone please distill the patches above? Or do I need to apply all of them? Additionally: if you're adding a new file (as it seems one of the patches does) please comment it correctly so that it doesn't look like a botched "diff". This looks like a job for super-Tholo... (cue theme music)
By: Roy Sigurd Karlsbakk (rkarlsba) 2004-05-04 04:33:32

Asterisk used to compile earlier. Has there been any major changes yet that need to be there and blocked portability?
By: Mark Spencer (markster) 2004-05-04 10:19:35

Yes, the changes are the ones in the title: gethostbyname_r has to be used because gethostbyname is not reentrant. No problem with this one, we have a gethostbyname_r for FreeBSD within Asterisk and it can be turned on to be used with MacOSX. What *is* a bigger problem is the transition to recursive mutexes, and if you can't find a way to do that, nothing past Asterisk 1.0 (the current stable branch) will ever compile on Mac OSX.
By: Roy Sigurd Karlsbakk (rkarlsba) 2004-05-04 10:29:09

Please apologize my missing know-how, but when are recursive mutexes required?
By: Olle Johansson (oej) 2004-05-04 10:57:03

Googling... Please read
http://archives.postgresql.org/pgsql-hackers/2003-06/msg01116.php

There's a code example in there...

http://force-elite.com/~chip/patches/ferite/ferite-freebsd-current-threads.patch
Patches that fakes recursive mutex for Darwin (OS/X) and Solaris
And uses recursive mutexes on FreeBSD

Code example on recursive locks on FreeBSD here:
http://www.ravenbrook.com/project/mps/master/code/lockfr.c

Quote:
"Currently in FreeBSD, mutexes are not recursive by default. A mutex can be made recursive by passing a flag to mtx_init. Exclusive sx locks never allow recursion, but shared sx locks always allow recursion. If a thread attempts to recursively lock a non-recursive lock, the kernel will panic reporting the file and line number of the offending lock operation"
http://www.usenix.org/events/bsdcon02/full_papers/baldwin/baldwin_html/node6.html
(Dated 2001)

" # Check to see if recursive mutexes are available
AC_MSG_CHECKING(for recursive mutexes)
has_recursive_mutexes=no
AC_TRY_LINK([
#include <pthread.h>
],[
pthread_mutexattr_t attr;
#if defined(linux) && !(defined(__arm__) && defined(QWS))
pthread_mutexattr_setkind_np(&attr, PTHREAD_MUTEX_RECURSIVE_NP);
#else
pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
#endif
],[
has_recursive_mutexes=yes
])
# Some systems have broken recursive mutex implementations
case "$target" in
*-*-darwin*)
has_recursive_mutexes=no
;;
*-*-solaris*)
has_recursive_mutexes=no
;;
esac
"
From http://cvs.sourceforge.net/viewcvs.py/akarmi/akarmi01/configure.in?rev=1.32
By: Olle Johansson (oej) 2004-05-04 11:05:30

jtodd:
We've solved the gethostbyname_r problem in CVS. Now, I guess the OpenBSD patch is the best to use, also for FreeBSD. The problem is that this is a workaround, not a fix. We still neeed recursive mutexes on Free/OpenBSD. It's doable.

After googling, I've come to a conclusion that it's doable on *BSD, but that it doesn't seem doable on OS/X, even though it's based on FreeBSD. Maybe later versions have this, since they upgraded the FreeBSD parts of OS/X.

There's a lot of code out there to be inspired from, PostgreSQL, GTK and others have it. With a coder (tholo? :-) we can have it. I haven't got a clue on this level of coding...
By: Olle Johansson (oej) 2004-05-05 02:58:39

Tilghman on the mailing list:
"Just use PTHREAD_MUTEX_STATIC_INITIALIZER in FreeBSD. That's
the initializer that was used in the original stdtime/localtime.c
which came out of FreeBSD! I had to change the mutexes for Asterisk,
because at the time, Asterisk was using non-recursive mutexes. This
mutex for FreeBSD should therefore already be recursive."
By: Olle Johansson (oej) 2004-05-05 03:01:09

Markus Wild on the mailing list:

Uhm... you mean this one from <pthread.h> ?
#define PTHREAD_MUTEX_INITIALIZER NULL

The problem with this is: yes, it will initialize your mutex so you
can start to use it. No, it will not statically preset it to be
of PTHREAD_MUTEX_RECURSIVE type, but of PTHREAD_MUTEX_ERRORCHECK. Check
/usr/src/lib/libpthread/thread/thr_mutex.c:_pthread_mutex_init()

To properly solve this, we'll probably have to roll our own type
that will allow for static initialization including type. We could
probably also take care to actually implement recursive mutexes in
there for OS X.. (very similar approach to what is currently enabled
with -DDEBUG_THREADS).

To get it done quicker (not for OS X), the following will work for
the case where _only_ recursive mutexes are used, relying on
pthread_mutex_t being a pointer:

--- include/asterisk/lock.h 22 Apr 2004 00:20:34 -0000 1.16
+++ include/asterisk/lock.h 4 May 2004 21:22:12 -0000
@@ -79,6 +107,9 @@

static inline int __ast_pthread_mutex_lock(char *filename, int lineno, char *func, ast_mutex_t *t) {
int res;
+#ifdef __FreeBSD__
+ if (! t->mutex) ast_mutex_init(t);
+#endif
res = pthread_mutex_lock(&t->mutex);
if (!res) {
t->file = filename;
@@ -99,6 +130,9 @@

static inline int __ast_pthread_mutex_trylock(char *filename, int lineno, char *func, ast_mutex_t *t) {
int res;
+#ifdef __FreeBSD__
+ if (! t->mutex) ast_mutex_init(t);
+#endif
res = pthread_mutex_trylock(&t->mutex);
if (!res) {
t->file = filename;

Note: you need to have the optional

# Optional debugging parameters
DEBUG_THREADS = -DDEBUG_THREADS -DDO_CRASH

enabled in your Makefile for this to work.

Cheers,
Markus
By: Olle Johansson (oej) 2004-05-05 03:06:02

Reply from Tilghman:
-"No, I was referring to the code in:
/usr/src/lib/libc/stdtime/localtime.c, which is the original code for:
/usr/src/asterisk/stdtime/localtime.c.

There are a few changes I've made to that source, in order to have
it work in the Asterisk environment, but that's where I'm getting that
particular mutex initializer"
---------------------
Markus reply:
-"But you're not getting (and you don't need) _recursive_ mutexes there...
The problem currently is not whether we can statically initialize mutexes
(we can), the problem is we can't initialize them to a specific type."
---------------------
Tilghman reply:
Yes you do. If the TZ environmental variable is not set (and it is not set in the default environment), then lcl_mutex in localtime.c is called recursively.

edited on: 05-24-04 09:53
By: rich (rich) 2004-05-13 18:09:47

freebsd.diff is vs the current cvs head. I've tested zap, sip and iax2 on both Redhat 9 and FreeBSD 5.2.1 successfully with this diff. Cheers, Rich
By: rich (rich) 2004-05-13 18:16:15

Also, in the patch, #ifdef __FreeBSD__ should probably modified to cover darwin and openbsd, but I don't have them available to test.
By: Olle Johansson (oej) 2004-05-16 06:01:44

Rich's RECURSIVE_MUTEX patch compiles on my FreeBSD 4.9 system. Can't judge if it works or not, but it can't hurt including it into CVS :-)

Rich, a *BIG* thank you for clearing this up.
By: James Golovich (jamesgolovich) 2004-05-16 12:36:48

Just eyeballed the latest patch and it won't work on linux. The #ifdef needs to get changed to a #if defined(PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP). Thats the same thing I did way at the beginning of this thread
By: Mark Spencer (markster) 2004-05-16 13:48:55

I'm removing the notation that it doesn't support gethostbyname_r since we have a work around for that.
By: rich (rich) 2004-05-17 00:57:54

I'm working on inserting calls to pthread_mutex_init() for FreeBSD since it doesn't support static initialization. I haven't seen breakage becuase I'm running it on a uniprocessor at under 1% cpu utilization.
By: rich (rich) 2004-05-18 21:20:14

mutex.dif (I believe) takes care of initialization of all the mutexes. I've compiled and tested for sip and iax2 connections on linux and FreeBSD. However, it should be tested on a heavily loaded multiprocessor before we put any confidence in it.
By: jerjer (jerjer) 2004-05-18 22:20:48

I have hardware that I can dedicate to the task. Unfortunatly I only have Linux installed at this point.
By: James Golovich (jamesgolovich) 2004-05-18 22:33:44

Once again this will not work properly on linux:
+#ifdef PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
it should be #if defined(PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP) instead

I'm not convinced that there isn't an easier solution that could be implemented only modifying include/asterisk/lock.h and keeping all the rest of the code as is.
By: rich (rich) 2004-05-19 05:58:28

I've looked at all the headers (userland and kernel) as well as the sources for pthreads on FreeBSD. There does not seem to be something equivalent to PTHREAD_RECURSIVE_MUTEX_INITIALIZER for pthreads on FreeBSD.

I didn't see any obvious way to staticly initialize a recursive mutex using the exposed API/headers. If that's not correct, please feel free to point it out. I sure would have rather limited the changes to lock.h.

Linux should be adequate for testing since they are using the same lock initialization method per the API. Thanks JerJer!

I wasn't able to notice any problems with '#ifdef' vs '#if defined()' on redhat 9, so I'm unaware of the cause of this issue, but lock-h-ifdef.h makes this change.
By: rich (rich) 2004-05-19 06:22:00

It would be possible to limit the changes to lock.h if one was willing to accept some increased latency of locking. However, I believe that one of the explicit goals is to "avoid diminishing the performance on Linux whatsoever." The changes should have no impact on performance.
By: James Golovich (jamesgolovich) 2004-05-19 12:07:45

There shouldn't be any increased latency on linux (or anything else that has the _NP initializers). On Linux or any host with them we would use that. In cases where we don't have _NP then we can make AST_MUTEX_INITIALIZER be a struct that is basically uninitialized. (ast_mutex_t would be a struct in this case). In the ast_mutex_* calls there would be an #ifndef THREAD_NP then a check if the mutex has already been initialized and if not a call to ast_mutex_init

There would be no performance hit with anything that has _NP initializers, but on other hosts there would be an additional small check that shouldn't really hurt too much
By: rich (rich) 2004-05-19 12:31:25

It certainly would be simpler to wrap the lock structure and locking calls on FreeBSD, though even if the performance cost is negligilble at this time, I'd far prefer to be confident that FreeBSD has equal performance characteristics going forward. If performance matters on Linux, it would matter equally on FreeBSD I believe. Should any fine grained computation/communication occur, direct use of the locking API would be an advantage. My impression is that performance is a priority.
By: lwc (lwc) 2004-05-22 08:53:31

Hi folks,
forgive, but I'm (i) not sure what the status is on recursive initialisers for OSX for this bug, and (ii) don't know enough about this topic to tell if it's working or not.

Just browsing OSX 10.3.3 apropos page on pthread_mutex_init, I note that pthread_mutextattr_init man page describes (amongst others) setting the kind to recursive, and that one pthread_mutexattr object can be used for multiple calls to pthread_mutex_init.

Forgive lack of clue, but doesn't this mean that one has a single "predefined attributes" object (set to recursive), and then calls pthread_mutex_init with this whenever one wants to create a new mutex?
If so, then what's the problem (apart from having to select on OS for the particular calls to be made when setting up a mutex)?
atb, Lawrence
By: Tilghman Lesher (tilghman) 2004-05-22 10:46:29

Please see the uploaded file mutextest.c, which compiles and runs on Mac OS X. This should prove beyond ANY doubt whatsoever that Mac OS X does INDEED have recursive mutexes.
By: Mark Spencer (markster) 2004-05-24 00:07:55

Okay, Rich, I just ran a test on a FreeBSD machine with the following code:

#include <stdio.h>
#include <pthread.h>

int main(int argc, char *argv[])
{
pthread_mutex_t lock;
#if 0
pthread_mutexattr_t attr;
pthread_mutexattr_init(&attr);
pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
pthread_mutex_init(&lock, &attr);
#else
pthread_mutex_init(&lock, NULL);
#endif
printf("1. Lock\n");
pthread_mutex_lock(&lock);
printf("2. Recursive lock\n");
pthread_mutex_lock(&lock);
printf("3. Unlock\n");
pthread_mutex_unlock(&lock);
printf("4. Unlock Recursive\n");
pthread_mutex_unlock(&lock);
exit(0);
}

and it ran cleanly. Can you confirm? If so, this means FreeBSD *does* use recursive mutexes by default and this is all moot with respect to FreeBSD (although the test program does not run properly under OSX)
By: Mark Spencer (markster) 2004-05-24 02:10:13

#define PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP { 0x4d555458, \
{ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
0x20 } };

is experimentally an acceptable initializer under MacOS X

Updated patches anyone?
By: Tilghman Lesher (tilghman) 2004-05-24 02:25:34

Mark- was your FreeBSD test done on 4.9 or 5.2? It matters, as FreeBSD still considers 4.x stable and 5.x experimental.
By: Olle Johansson (oej) 2004-05-24 02:49:49

Mark's code runs cleanly on my FreeBSD 4.9 system.
By: Roy Sigurd Karlsbakk (rkarlsba) 2004-05-24 03:55:49

As run on 10.3.3:
Pearl:~/src roy$ ./mutextest
After first lock
Second lock succeeded
After second lock
After first unlock
After second unlock
By: rich (rich) 2004-05-24 08:14:42

Yep, it worked, but take a look at this example...

#include <stdio.h>
#include <pthread.h>

static pthread_t test_thread;
static pthread_mutex_t lock;

static void *testthread(void *data)
{
printf("1a. Lock\n");
pthread_mutex_lock(&lock);
printf("2a. Recursive lock\n");
pthread_mutex_lock(&lock);
printf("3a. Unlock\n");
pthread_mutex_unlock(&lock);
printf("4a. Unlock Recursive\n");
pthread_mutex_unlock(&lock);
}

int main(int argc, char *argv[])
{

pthread_mutex_init(&lock, NULL);

printf("1. Lock\n");
pthread_mutex_lock(&lock);
printf("2. Recursive lock\n");
pthread_mutex_lock(&lock);
printf("3. Unlock\n");
pthread_mutex_unlock(&lock);
pthread_create(&test_thread, NULL, testthread, NULL);
pthread_yield();
usleep(100);
printf("4. Unlock Recursive\n");
pthread_mutex_unlock(&lock);
exit(0);
}

produces

gcc -pthread asdf.c
./a.out
1. Lock
2. Recursive lock
3. Unlock
1a. Lock
2a. Recursive lock
3a. Unlock
4a. Unlock Recursive
4. Unlock Recursive

I believe that the default locks don't do the counting that the recursive locks do.
By: Roy Sigurd Karlsbakk (rkarlsba) 2004-05-24 09:35:11

What's wrong here?

Pearl:~/src roy$ cc -o mutextest mutextest2.c -lpthread
ld: warning prebinding disabled because of undefined symbols
ld: Undefined symbols:
_pthread_yield
By: Tilghman Lesher (tilghman) 2004-05-24 10:50:35

oej and rkarlsba: were you both running the mutextest.c code above? Or the code that Mark put into the bugnote? mutextest.c is mine, not Mark's, and its success does not tell you anything about Mark's code. rkarlsba's output suggests that he was running my test code, not Mark's.

Rich: you're correct; fast mutexes do not count, as locking more than once will result in a deadlock (therefore, it doesn't make any sense for them to count).
By: Mark Spencer (markster) 2004-05-24 10:59:03

Yes, but fast mutexes don't let you grab them more than once, too. Rich's test is very strange indeed because it's allowing the lock to be grabbed twice, but once unlocked, isn't requiring it to be unlocked a second time before being locked by the other thread. Did you test to see that initializing it as a recursive one fixes that issue?

Ironic, but it seems we can support MacOS sooner than FreeBSD perhaps?
By: Mark Spencer (markster) 2004-05-24 11:17:10

I've added a preliminary patch for MacOS so at least the recursive mutexes should pass.
By: Olle Johansson (oej) 2004-05-24 11:23:46

Mutextest.c also runs fine on FreeBSD 4.9

OS/X is based on FreeBSD, so they should pretty much be alike.
By: rich (rich) 2004-05-24 11:44:36

Here's the behavior with a recursive mutex:
#include <stdio.h>
#include <pthread.h>

static pthread_t test_thread;
static pthread_mutex_t lock;

static void *testthread(void *data)
{
printf("1a. Lock\n");
pthread_mutex_lock(&lock);
printf("2a. Recursive lock\n");
pthread_mutex_lock(&lock);
printf("3a. Unlock\n");
pthread_mutex_unlock(&lock);
printf("4a. Unlock Recursive\n");
pthread_mutex_unlock(&lock);
}

int main(int argc, char *argv[])
{
pthread_mutexattr_t attr;
pthread_mutexattr_init(&attr);
pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
pthread_mutex_init(&lock, &attr);

printf("1. Lock\n");
pthread_mutex_lock(&lock);
printf("2. Recursive lock\n");
pthread_mutex_lock(&lock);
printf("3. Unlock\n");
pthread_mutex_unlock(&lock);
pthread_create(&test_thread, NULL, testthread, NULL);
pthread_yield();
usleep(100);
printf("4. Unlock Recursive\n");
pthread_mutex_unlock(&lock);
usleep(1000);
exit(0);
}

gcc -pthread asdf.c
./a.out
1. Lock
2. Recursive lock
3. Unlock
1a. Lock
4. Unlock Recursive
2a. Recursive lock
3a. Unlock
4a. Unlock Recursive
By: Olle Johansson (oej) 2004-05-24 12:02:47

Rich code works on FreeBSD 4.9... I don't know what I'm doing, just adding to confusion or evidence - but testing all code snippets I find :-9

primula# gcc -pthread rich.c -o rich
primula# ./rich
1. Lock
2. Recursive lock
3. Unlock
1a. Lock
4. Unlock Recursive
2a. Recursive lock
3a. Unlock
4a. Unlock Recursive
By: rich (rich) 2004-05-24 12:25:26

The above shows that the default lock type is not recursive.

pthread_mutex_t is a pointer to a mutex lock structure. The static initializer simply sets the value of the pointer to NULL. lock() does a runtime check for this null pointer value that then allocates memory and initializes that structure. See:

/usr/src/lib/libpthread/thread/thr_mutex.c

It's certainly inconvenient, but on the bright side, the sources provide some clear statement about what's possible or not possible.

Bottom line: static initialization of a recursive mutex isn't possible. The default mutex type isn't really statically initialized either, it's just a special case that is initialized on first use. Because it's a special case, there is no way to take advantage of the initialization-on-first-use for recursive mutexes.

There is however some hope if you're willing to use gcc specific extensions. More in the next message....
By: rich (rich) 2004-05-24 12:34:14

Here's an example of file scope initialization for the recursive mutexes:

#include <stdio.h>
#include <pthread.h>

static pthread_t test_thread;
static pthread_mutex_t lock;

static void thunk(void) __attribute__ ((constructor));

static void thunk(void)
{
pthread_mutexattr_t attr;
pthread_mutexattr_init(&attr);
pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
pthread_mutex_init(&lock, &attr);
}

static void *testthread(void *data)
{
printf("1a. Lock\n");
pthread_mutex_lock(&lock);
printf("2a. Recursive lock\n");
pthread_mutex_lock(&lock);
printf("3a. Unlock\n");
pthread_mutex_unlock(&lock);
printf("4a. Unlock Recursive\n");
pthread_mutex_unlock(&lock);
}

int main(int argc, char *argv[])
{
printf("1. Lock\n");
pthread_mutex_lock(&lock);
printf("2. Recursive lock\n");
pthread_mutex_lock(&lock);
printf("3. Unlock\n");
pthread_mutex_unlock(&lock);
pthread_create(&test_thread, NULL, testthread, NULL);
pthread_yield();
usleep(100);
printf("4. Unlock Recursive\n");
pthread_mutex_unlock(&lock);
usleep(1000);
exit(0);
}

gcc -pthread asdf.c
./a.out
1. Lock
2. Recursive lock
3. Unlock
1a. Lock
4. Unlock Recursive
2a. Recursive lock
3a. Unlock
4a. Unlock Recursive

I'd be glad to rewrite the patch using this method if that's acceptable.
By: Tilghman Lesher (tilghman) 2004-05-24 13:15:15

Rich: could you rewrite to detect that the second lock is INDEED succeeding (i.e. return value)? The way it's written now, it's possible that the second lock fails due to a deadlock being averted.

If you did not have recursive locks here, then your program should freeze at the time of the second pthread_mutex_lock() -- because it's stopped, waiting for a lock to be freed that will never be freed (because the thread itself is the holder of the lock -- deadlock).
By: rich (rich) 2004-05-24 13:57:18

Note that this is the desired behavior. A given thread is able to repeatedly acquire locks that it currently holds, and after all of the corresponding unlock()s have occurred, any other waiting threads are able to acquire the lock and continue execution.

That is, the thread must wait till main has finished both unlock()s before it can execute.

gcc -pthread asdf.c
./a.out
Main - 1. Waiting for 1st lock
Main - 2. Got 1st lock, waiting for 2nd (recursive) lock
Main - 3. Got 2nd lock
Thread - 1. Waiting for 1st lock
Main - 3a. Unlocking 1st time
Main - 4. Unlocked 1st time, Unlocking 2nd time:
Main - 5. Unlocked 2nd time. Done.
Thread - 2. Got 1st lock, waiting for 2nd (recursive) lock
Thread - 3. Got 2nd lock, Unlocking 1st time:
Thread - 4. Unlocked 1st time, Unlocking 2nd time:
Thread - 5. Unlocked 2nd time. Done.

#include <stdio.h>
#include <pthread.h>

static pthread_t test_thread;
static pthread_mutex_t lock;

static void __attribute__ ((constructor)) thunk(void)
{
pthread_mutexattr_t attr;
pthread_mutexattr_init(&attr);
pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
pthread_mutex_init(&lock, &attr);
}

static void *testthread(void *data)
{
int ret;
printf("Thread - 1. Waiting for 1st lock\n");
if(pthread_mutex_lock(&lock))
perror("Thread - 1st lock");
printf("Thread - 2. Got 1st lock, waiting for 2nd (recursive) lock\n");
if(pthread_mutex_lock(&lock))
perror("Thread - 2nd lock");
printf("Thread - 3. Got 2nd lock, Unlocking 1st time:\n");
if(pthread_mutex_unlock(&lock))
perror("Thread - 1st unlock");
printf("Thread - 4. Unlocked 1st time, Unlocking 2nd time:\n");
if(pthread_mutex_unlock(&lock))
perror("Thread - 2nd unlock");
printf("Thread - 5. Unlocked 2nd time. Done.\n");
}

int main(int argc, char *argv[])
{
int ret = 0;
printf("Main - 1. Waiting for 1st lock\n");
if(pthread_mutex_lock(&lock))
perror("Main - 1st lock");
printf("Main - 2. Got 1st lock, waiting for 2nd (recursive) lock\n");
if(pthread_mutex_lock(&lock))
perror("Main - 2nd lock");
printf("Main - 3. Got 2nd lock\n");
pthread_create(&test_thread, NULL, testthread, NULL);
pthread_yield();
usleep(100);
printf("Main - 3a. Unlocking 1st time\n");
if(pthread_mutex_unlock(&lock))
perror("Main - 1st unlock");
pthread_yield();
usleep(100);
printf("Main - 4. Unlocked 1st time, Unlocking 2nd time:\n");
if(pthread_mutex_unlock(&lock))
perror("Main - 2nd unlock");
printf("Main - 5. Unlocked 2nd time. Done.\n");
usleep(1000);
exit(0);
}
By: Olle Johansson (oej) 2004-05-24 14:04:01

The code in the bugnote above works on FreeBSD 4.9. :-)
By: Mark Spencer (markster) 2004-05-24 14:09:25

IT could be that they are error checking mutexes instead of recursive ones.
By: rich (rich) 2004-05-24 14:24:29

Here's the behavior for error checking (default) mutexes.

This isn't the desired behavior because the tread doesn't wait till main() has finished the 2nd unlock().

gcc -pthread asdf.c
./a.out
Main - 1. Waiting for 1st lock
Main - 2. Got 1st lock, waiting for 2nd (recursive) lock
Main - 2nd lock: Unknown error: 0
Main - 3. Got 2nd lock
Thread - 1. Waiting for 1st lock
Main - 3a. Unlocking 1st time
Thread - 2. Got 1st lock, waiting for 2nd (recursive) lock
Thread - 2nd lock: Unknown error: 0
Thread - 3. Got 2nd lock, Unlocking 1st time:
Thread - 4. Unlocked 1st time, Unlocking 2nd time:
Thread - 2nd unlock: Unknown error: 0
Thread - 5. Unlocked 2nd time. Done.
Main - 4. Unlocked 1st time, Unlocking 2nd time:
Main - 2nd unlock: Unknown error: 0
Main - 5. Unlocked 2nd time. Done.

#include <stdio.h>
#include <pthread.h>

static pthread_t test_thread;
static pthread_mutex_t lock;

static void __attribute__ ((constructor)) thunk(void)
{
#if 0
pthread_mutexattr_t attr;
pthread_mutexattr_init(&attr);
pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
pthread_mutex_init(&lock, &attr);
#endif
}

static void *testthread(void *data)
{
int ret;
printf("Thread - 1. Waiting for 1st lock\n");
if(pthread_mutex_lock(&lock))
perror("Thread - 1st lock");
printf("Thread - 2. Got 1st lock, waiting for 2nd (recursive) lock\n");
if(pthread_mutex_lock(&lock))
perror("Thread - 2nd lock");
printf("Thread - 3. Got 2nd lock, Unlocking 1st time:\n");
if(pthread_mutex_unlock(&lock))
perror("Thread - 1st unlock");
printf("Thread - 4. Unlocked 1st time, Unlocking 2nd time:\n");
if(pthread_mutex_unlock(&lock))
perror("Thread - 2nd unlock");
printf("Thread - 5. Unlocked 2nd time. Done.\n");
}

int main(int argc, char *argv[])
{
int ret = 0;
printf("Main - 1. Waiting for 1st lock\n");
if(pthread_mutex_lock(&lock))
perror("Main - 1st lock");
printf("Main - 2. Got 1st lock, waiting for 2nd (recursive) lock\n");
if(pthread_mutex_lock(&lock))
perror("Main - 2nd lock");
printf("Main - 3. Got 2nd lock\n");
pthread_create(&test_thread, NULL, testthread, NULL);
pthread_yield();
usleep(100);
printf("Main - 3a. Unlocking 1st time\n");
if(pthread_mutex_unlock(&lock))
perror("Main - 1st unlock");
pthread_yield();
usleep(100);
printf("Main - 4. Unlocked 1st time, Unlocking 2nd time:\n");
if(pthread_mutex_unlock(&lock))
perror("Main - 2nd unlock");
printf("Main - 5. Unlocked 2nd time. Done.\n");
usleep(1000);
exit(0);
}
By: Olle Johansson (oej) 2004-05-24 14:27:51

As the code grows, could you please upload files as files in the tracker instead of including it in the bug note? Thank you! /Olle
By: rich (rich) 2004-05-24 14:38:57

Note that this uses only documented posix and gcc features. So, in principle it should be portable to those platforms that support gcc and posix recursive mutexes.
By: rich (rich) 2004-05-27 08:25:55

gcc's __attribute__ should help isolate changes and elminate the source file interdependency created in mutex.dif.

Would that change be sufficient, or are there other issues to solve?
By: rich (rich) 2004-05-28 08:59:54

Please tell me what you want in terms of a patch and I'll send it! Thanks, Rich
By: rich (rich) 2004-05-29 10:17:47

Myself and Rich Neese have tested code using the 'constructor' to initialize recursive mutexes. It seems to be stable in terms of up-time and functionality.

James and Mark, can you give me some feedback abou this?

Thanks,
Rich
By: rich (rich) 2004-06-03 01:54:18

mutex3.dif is a revised patch that should leave the code unchanged on linux. On FreeBSD, it uses constructor/destructors to initialize mutexes when compiled by gcc. A third alternative is implmented, initialization on first use, for compilers other than gcc.

I've tested it on Redhat 9 and FreeBSD 5.2.1.

This includes all the changes I've discussed with Mark and I hope addressess all the issues raised.

Cheers,
Rich
By: Mark Spencer (markster) 2004-06-03 14:47:05

One last suggestion: We don't want anyone using AST_MUTEX_INITIALIZER anymore, so internally call it something else, and then we can define AST_MUTEX_INTIALIZER to be __USE_AST_MUTEX_DEFINE_INSTEAD_OF_AST_MUTEX_INITIALIZER__ so that code that uses AST_MUTEX_INITIALIZER directly will fail.

Did you test this with both DEBUG_THREADS and without?
By: rich (rich) 2004-06-03 17:33:01

mutex5.dif takes care of all those I hope, including the define for AST_MUTEX_INTIALIZER, and testing with and without DEBUG_THREADS on redhat 9 and freebsd 5.2.1.
By: Mark Spencer (markster) 2004-06-08 20:42:45

Added to CVS, thanks for your contribution!
By: Digium Subversion (svnbot) 2008-01-15 14:52:16.000-0600

Repository: asterisk
Revision: 2793

U trunk/asterisk.c
U trunk/channels/chan_zap.c

------------------------------------------------------------------------
r2793 | markster | 2008-01-15 14:52:16 -0600 (Tue, 15 Jan 2008) | 2 lines

Provide gethostbyname_r emulation for FreeBSD and fix zap call to setstate to include callerid (bug ASTERISK-1396, ASTERISK-1483)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=2793
By: Digium Subversion (svnbot) 2008-01-15 14:57:43.000-0600

Repository: asterisk
Revision: 3176

U trunk/acl.c
U trunk/apps/app_intercom.c
U trunk/apps/app_meetme.c
U trunk/apps/app_queue.c
U trunk/apps/app_voicemail.c
U trunk/asterisk.c
U trunk/astmm.c
U trunk/autoservice.c
U trunk/cdr/cdr_odbc.c
U trunk/cdr/cdr_pgsql.c
U trunk/cdr.c
U trunk/channel.c
U trunk/channels/chan_agent.c
U trunk/channels/chan_alsa.c
U trunk/channels/chan_h323.c
U trunk/channels/chan_iax.c
U trunk/channels/chan_iax2.c
U trunk/channels/chan_local.c
U trunk/channels/chan_mgcp.c
U trunk/channels/chan_modem.c
U trunk/channels/chan_modem_aopen.c
U trunk/channels/chan_modem_bestdata.c
U trunk/channels/chan_modem_i4l.c
U trunk/channels/chan_nbs.c
U trunk/channels/chan_oss.c
U trunk/channels/chan_phone.c
U trunk/channels/chan_sip.c
U trunk/channels/chan_skinny.c
U trunk/channels/chan_vofr.c
U trunk/channels/chan_vpb.c
U trunk/channels/chan_zap.c
U trunk/cli.c
U trunk/codecs/codec_a_mu.c
U trunk/codecs/codec_adpcm.c
U trunk/codecs/codec_alaw.c
U trunk/codecs/codec_g723_1.c
U trunk/codecs/codec_g726.c
U trunk/codecs/codec_gsm.c
U trunk/codecs/codec_ilbc.c
U trunk/codecs/codec_lpc10.c
U trunk/codecs/codec_speex.c
U trunk/codecs/codec_ulaw.c
U trunk/db.c
U trunk/dns.c
U trunk/enum.c
U trunk/file.c
U trunk/formats/format_g723.c
U trunk/formats/format_g726.c
U trunk/formats/format_g729.c
U trunk/formats/format_gsm.c
U trunk/formats/format_h263.c
U trunk/formats/format_ilbc.c
U trunk/formats/format_pcm.c
U trunk/formats/format_pcm_alaw.c
U trunk/formats/format_vox.c
U trunk/formats/format_wav.c
U trunk/formats/format_wav_gsm.c
U trunk/frame.c
U trunk/image.c
U trunk/include/asterisk/linkedlists.h
U trunk/include/asterisk/lock.h
U trunk/include/asterisk/module.h
U trunk/indications.c
U trunk/loader.c
U trunk/logger.c
U trunk/manager.c
U trunk/pbx/pbx_config.c
U trunk/pbx/pbx_gtkconsole.c
U trunk/pbx.c
U trunk/res/res_crypto.c
U trunk/res/res_monitor.c
U trunk/res/res_musiconhold.c
U trunk/res/res_parking.c
U trunk/stdtime/localtime.c
U trunk/translate.c
U trunk/utils.c

------------------------------------------------------------------------
r3176 | markster | 2008-01-15 14:57:42 -0600 (Tue, 15 Jan 2008) | 2 lines

Merge FreeBSD locking fixes (bug ASTERISK-1396)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=3176