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:
|
/* 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:
I've noticed a potential issue with the
bsem_post_allfunction. It appears that this function may not be working as intended and is essentially equivalent tobsem_post. Here's my analysis:C-Thread-Pool/thpool.c
Lines 545 to 571 in 4eb5a69
1.
bsem_post_allonly setbsem_p->v = 1once.2. Although it uses
pthread_cond_broadcast, multiple threads waiting inbsem_waitwould check ifbsem_p->vis set to 1 before they leavebsem_wait.3. If a thread leaves
bsem_waitsuccessfully, it will setbsem_p->v = 0, preventing other threads from leavingbsem_wait.As a result, using one
bsem_post_allstill only releases one thread waiting inbsem_wait.bsem_post_allis functionally equivalent tobsem_post, possibly consuming more resources due to the broadcast.In thpool_destroy, looping bsem_post should be sufficient and potentially more efficient: