-
Notifications
You must be signed in to change notification settings - Fork 634
Description
I've noticed a potential issue with the bsem_post_all
function. It appears that this function may not be working as intended and is essentially equivalent to bsem_post
. Here's my analysis:
Lines 545 to 571 in 4eb5a69
/* Post to at least one thread */ | |
static void bsem_post(bsem *bsem_p) { | |
pthread_mutex_lock(&bsem_p->mutex); | |
bsem_p->v = 1; | |
pthread_cond_signal(&bsem_p->cond); | |
pthread_mutex_unlock(&bsem_p->mutex); | |
} | |
/* Post to all threads */ | |
static void bsem_post_all(bsem *bsem_p) { | |
pthread_mutex_lock(&bsem_p->mutex); | |
bsem_p->v = 1; | |
pthread_cond_broadcast(&bsem_p->cond); | |
pthread_mutex_unlock(&bsem_p->mutex); | |
} | |
/* Wait on semaphore until semaphore has value 0 */ | |
static void bsem_wait(bsem* bsem_p) { | |
pthread_mutex_lock(&bsem_p->mutex); | |
while (bsem_p->v != 1) { | |
pthread_cond_wait(&bsem_p->cond, &bsem_p->mutex); | |
} | |
bsem_p->v = 0; | |
pthread_mutex_unlock(&bsem_p->mutex); | |
} |
1.bsem_post_all
only set bsem_p->v = 1
once.
2. Although it uses pthread_cond_broadcast
, multiple threads waiting in bsem_wait
would check if bsem_p->v
is set to 1 before they leave bsem_wait
.
3. If a thread leaves bsem_wait
successfully, it will set bsem_p->v = 0
, preventing other threads from leaving bsem_wait
.
As a result, using one bsem_post_all
still only releases one thread waiting in bsem_wait
. bsem_post_all
is functionally equivalent to bsem_post
, possibly consuming more resources due to the broadcast.
In thpool_destroy, looping bsem_post should be sufficient and potentially more efficient: