STR #1575

FLTK matrix user chat room
(using Element browser app)   FLTK gitter user chat room   GitHub FLTK Project   FLTK News RSS Feed  
  FLTK Apps      FLTK Library      Forums      Links     Login 
 Home  |  Articles & FAQs  |  Bugs & Features  |  Documentation  |  Download  |  Screenshots  ]
 

Return to Bugs & Features | Roadmap 1.1 ]

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:

Receive EMails Don't Receive EMails

Trouble Report Files:


Name/Time/Date Filename/Size top right image
 
#1 wavexx
13:02 Jan 27, 2007
recursive.patch
3k
 
bottom left image   bottom right image

Trouble Report Comments:


Name/Time/Date Text top right image
 
#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...  
bottom left image   bottom right image

Return to Bugs & Features ]

 
 

Comments are owned by the poster. All other content is copyright 1998-2022 by Bill Spitzak and others. This project is hosted by The FLTK Team. Please report site problems to 'erco@seriss.com'.