On Sat, Apr 18, 2020 at 3:29 AM Steve Williams < [email protected]> wrote:
> Hi, > > I apologize if this is not an appropriate forum to be posting this. > > I am looking into guacamole's use of pthread_mutexattr_setpshare. > > I am trying to port/compile guacamole 1.1.0 on OpenBSD. The only > (significant) errors I am getting pertain to the use of > pthread_mutexattr_setpshared(foo, PTHREAD_PROCESS_SHARED). > > If I comment out the 4 lines of code invoking > pthread_mutexattr_setpshare in the source tree, I can it to compile with > gcc on OpenBSD 6.6 (GENERIC.MP) ) > > I am an experienced C programmer, but I've never looked in threading in > C before so it has required quite a bit of reading. I haven't been able > to find a comprehensive "architecture" document for pthreads, just > various man pages, some with sample code. > > For example: > https://linux.die.net/man/3/pthread_mutexattr_init > > In the above documentation, it states: > > ...the possibility that an application may allocate the > synchronization objects from this section in memory that is accessed > by multiple processes (and therefore, by threads of multiple > processes). > > It appears that the purpose of the guacamole's > pthread_mutexattr_setpshared(foo, PTHREAD_PROCESS_SHARED) is to permit > multiple threaded processes to access the resource protected by the MUTEX. > > However, from my reading of the guacamole code, the MUTEX is only > protecting malloc'd memory, which as far as I know, isn't a resource > that can be accessed by multiple processes. > > It is my newby (to pthreads) interpretation of the code that in all 4 > cases, the code is malloc'ing memory that is being protected by the > MUTEX. For example: > > src/libguac/pool.c: > ... > guac_pool* guac_pool_alloc(int size) { > > pthread_mutexattr_t lock_attributes; > guac_pool* pool = malloc(sizeof(guac_pool)); > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > Allocate memory that will only be accessible by this process > > /* If unable to allocate, just return NULL. */ > if (pool == NULL) > return NULL; > > /* Initialize empty pool */ > pool->min_size = size; > pool->active = 0; > pool->__next_value = 0; > pool->__head = NULL; > pool->__tail = NULL; > > /* Init lock */ > pthread_mutexattr_init(&lock_attributes); > pthread_mutexattr_setpshared(&lock_attributes, > PTHREAD_PROCESS_SHARED); > pthread_mutex_init(&(pool->__lock), &lock_attributes); > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > This codes saves the MUTEX in the locally allocated memory > > No other process can find this MUTEX in malloc'd memory, so setting it > to PTHREAD_PROCESS_SHARED seems totally irrelevant. > > If so, I *think* it's OK to just comment out the > pthread_mutexattr_setpshared code as OpenBSD's pthread MUTEX locking > implementation will work fine within a process, just not multiple > processes. > > So, the only thing that I wonder that this might impact would be connection sharing. I don't know for sure that this is the case - it may be that the connection sharing is done in such a way that doesn't require this, but based on my knowledge of how Guacamole works and the connection sharing involving someone initiating another connection to an existing running connection (=process) in guacd, it seems like this could break that or at the very least cause some undesirable behavior. I could be off on that - I'd be interested to see what others have to say about that - it's just my initial thought without diving into the code, and based on my fairly limited knowledge of pthreads. -Nick
