| | [ Return to Bugs & Features | Roadmap 1.1 | SVN ⇄ GIT ]
STR #1575
| Application: | FLTK Library |
|---|
| Status: | 1 - Closed w/Resolution |
|---|
| Priority: | 2 - Low, e.g. a documentation error or undocumented side-effect |
|---|
| Scope: | 3 - Applies to all machines and operating systems |
|---|
| Subsystem: | Core Library |
|---|
| Summary: | bad initialization/testing of recursive mutexes |
|---|
| Version: | 1.1-current |
|---|
| Created By: | wavexx |
|---|
| Assigned To: | mike |
|---|
| Fix Version: | 1.1-current (SVN: v5648) |
|---|
| Update Notification: | |
|---|
Trouble Report Files:
Trouble Report Comments:
| |
| #1 | wavexx
04:58 Jan 26, 2007 |
| From Chris:
Hello,
I would like to compile the fltk 1.1.x-r5622 with pthread's recursive mutexes.
When I examine Fl_lock.cxx, I see that pthread's recursive mutexes are only used if the following is true:
# if defined (PTHREAD_MUTEX_RECURSIVE_NP)
However, on my system (specs in sig), the type is PTHREAD_MUTEX_RECURSIVE. When I change the define to
look for PTHREAD_MUTEX_RECURSIVE pthread's recursive mutex is not compiled. I had to change the define to #if 1
Also, I believe that how the Fl_Mutex_attr is initialised is incorrect. The type of the mutex should be set using the
pthread_mutexattr_settype() function as in:
pthread_mutexattr_settype(&Fl_Mutex_attrib, PTHREAD_MUTEX_RECURSIVE);
I changed the lock_function() code in Fl_lock.cxx [starting at line 141] to the following in order for fltk to compile.
pthread_mutexattr_t Fl_Mutex_attrib;
static void lock_function() {
if (!minit) {
memset(&Fl_Mutex_attrib, 0, sizeof(pthread_mutexattr_t));
pthread_mutexattr_init(&Fl_Mutex_attrib);
pthread_mutexattr_settype(&Fl_Mutex_attrib, PTHREAD_MUTEX_RECURSIVE);
pthread_mutex_init(&fltk_mutex, &Fl_Mutex_attrib);
minit = true;
}
pthread_mutex_lock(&fltk_mutex);
}
Perhaps the system libraries or pthread implementation has changed since this part of the Fl_lock.cxx was written?
--
The original code seemed to exploit some specific version of pthreads.
Checking 'PTHREAD_MUTEX_RECURSIVE' at compile time however isn't enough.
settype may return EINVAL if the current kernel/implementation has no
support for it.
--
lock_function should check for settype return code and assign fl_lock_function correctly if PTHEAD_MUTEX_RECURSIVE is defined. | |
| |
| #2 | wavexx
13:02 Jan 27, 2007 |
| Attaching fix. | |
| |
| #3 | mike
12:27 Jan 28, 2007 |
| I'm inclined to drop the recursive mutex code and just use the code we already have for non-recursive mutex's - simpler, more portable, and well tested.
Let me know if the change has any (bad) side-effects for you... | |
| |
| #4 | wavexx
13:46 Jan 28, 2007 |
| I've been using the non-recursive version thinking of using the recursive one until I noticed Chris' message.
Not really an issue, but using the recursive one if available would be really nice. Maybe this can be postponed for 1.1.9 instead of dropping it completely. The change is not complicated. | |
| |
| #5 | mike
14:02 Jan 28, 2007 |
| What (real) advantage is there to using the recursive mutex, given that the usage of the mutex in FLTK is really not recursive to begin with? | |
| |
| #6 | wavexx
02:18 Jan 29, 2007 |
| For FLTK internal usage, none. But it may improve latency slightly if the FLTK lock is used as a main recursive mutex in the application.
How many platforms lack posix recursive mutexes? | |
| |
| #7 | mike
06:13 Jan 29, 2007 |
| All recent OS's that support POSIX threading also support recursive mutexes. The issue is whether a static initializer is supported...
Anyways, I'll try incorporating your patch (it doesn't want to apply cleanly to r5646) and test on all our lab systems when I'm back in the office in a few weeks... | |
| |
| #8 | mike
09:16 Jan 29, 2007 |
| OK, I've merged your patch in r5648; please let me know if I messed anything up (seems to work with the threads demo on OSX) | |
| |
| #9 | wavexx
13:10 Jan 30, 2007 |
| Mmh, I was under the impression that we were supporting a POSIX platform without recursive mutex support. In that case, wouldn't be better to remove entirely the non-recursive version? It would also save a pointer indirection.
The merge is fine. | |
| |
| #10 | mike
15:02 Jan 31, 2007 |
| I'm a little confused by your comment (missing word?), but in any case we can't guarantee support for recursive mutexes and so we still need the extra code if we want to take advantage of them. The amount of code is fortunately minimal... | |
[ Return to Bugs & Features ]
|
| |
