Skip to content

Commit f66baab

Browse files
IlanTruanovskypcolberg
authored andcommitted
Remove trylock + unlock in acl_reset_condvar()
See #276 for an explanation on why this needs to be done. In short, assuming library destructors work as we expect (i.e. they do not get called on abort), this code is unnecessary and can introduce undefined behavior. This will also fix the following Coverity issues: ``` lib/acl_threadsupport/test/acl_threadsupport_test.cpp:333:3: Type: Double unlock (LOCK) lib/acl_threadsupport/test/acl_threadsupport_test.cpp:332:3: 1. unlock: "acl_release_condvar" unlocks "cc.waiter_mutex". lib/acl_threadsupport/src/acl_threadsupport.c:375:5: 1.1. unlock: "pthread_mutex_unlock" unlocks "C->waiter_mutex". lib/acl_threadsupport/test/acl_threadsupport_test.cpp:333:3: 2. double_unlock: "acl_reset_condvar" unlocks "cc.waiter_mutex" while it is unlocked. lib/acl_threadsupport/src/acl_threadsupport.c:300:5: 2.1. unlock: "pthread_mutex_unlock" unlocks "C->waiter_mutex". lib/acl_threadsupport/src/acl_threadsupport.c:301:5: 2.2. destroy: "pthread_mutex_destroy" destroys "C->waiter_mutex". lib/acl_threadsupport/test/acl_threadsupport_test.cpp:353:3: Type: Double unlock (LOCK) lib/acl_threadsupport/test/acl_threadsupport_test.cpp:345:3: 1. path: Condition "i < 10000000U /* COUNT */", taking true branch. lib/acl_threadsupport/test/acl_threadsupport_test.cpp:346:5: 2. path: Condition "0 == i % (1000000U /* COUNT / 10 */)", taking true branch. lib/acl_threadsupport/test/acl_threadsupport_test.cpp:352:3: 3. path: Jumping back to the beginning of the loop. lib/acl_threadsupport/test/acl_threadsupport_test.cpp:345:3: 4. path: Condition "i < 10000000U /* COUNT */", taking true branch. lib/acl_threadsupport/test/acl_threadsupport_test.cpp:346:5: 5. path: Condition "0 == i % (1000000U /* COUNT / 10 */)", taking false branch. lib/acl_threadsupport/test/acl_threadsupport_test.cpp:351:5: 6. unlock: "acl_release_condvar" unlocks "cc.waiter_mutex". lib/acl_threadsupport/src/acl_threadsupport.c:375:5: 6.1. unlock: "pthread_mutex_unlock" unlocks "C->waiter_mutex". lib/acl_threadsupport/test/acl_threadsupport_test.cpp:352:3: 7. path: Jumping back to the beginning of the loop. lib/acl_threadsupport/test/acl_threadsupport_test.cpp:345:3: 8. path: Condition "i < 10000000U /* COUNT */", taking false branch. lib/acl_threadsupport/test/acl_threadsupport_test.cpp:353:3: 9. double_unlock: "acl_reset_condvar" unlocks "cc.waiter_mutex" while it is unlocked. lib/acl_threadsupport/src/acl_threadsupport.c:300:5: 9.1. unlock: "pthread_mutex_unlock" unlocks "C->waiter_mutex". lib/acl_threadsupport/src/acl_threadsupport.c:301:5: 9.2. destroy: "pthread_mutex_destroy" destroys "C->waiter_mutex". lib/acl_threadsupport/test/acl_threadsupport_test.cpp:477:3: Type: Double unlock (LOCK) lib/acl_threadsupport/test/acl_threadsupport_test.cpp:464:3: 1. path: Condition "1 != acl_timed_wait_condvar(&cc, 1)", taking false branch. lib/acl_threadsupport/test/acl_threadsupport_test.cpp:465:3: 2. path: Condition "1 != cc.timedout[0]", taking true branch. lib/acl_threadsupport/test/acl_threadsupport_test.cpp:465:3: 3. path: Falling through to end of if statement. lib/acl_threadsupport/test/acl_threadsupport_test.cpp:473:3: 4. path: Condition "0 != acl_timed_wait_condvar(&cc, 1)", taking true branch. lib/acl_threadsupport/test/acl_threadsupport_test.cpp:473:3: 5. path: Falling through to end of if statement. lib/acl_threadsupport/test/acl_threadsupport_test.cpp:474:3: 6. path: Condition "0 != cc.timedout[1]", taking true branch. lib/acl_threadsupport/test/acl_threadsupport_test.cpp:474:3: 7. path: Falling through to end of if statement. lib/acl_threadsupport/test/acl_threadsupport_test.cpp:475:3: 8. unlock: "acl_release_condvar" unlocks "cc.waiter_mutex". lib/acl_threadsupport/src/acl_threadsupport.c:375:5: 8.1. unlock: "pthread_mutex_unlock" unlocks "C->waiter_mutex". lib/acl_threadsupport/test/acl_threadsupport_test.cpp:477:3: 9. double_unlock: "acl_reset_condvar" unlocks "cc.waiter_mutex" while it is unlocked. lib/acl_threadsupport/src/acl_threadsupport.c:300:5: 9.1. unlock: "pthread_mutex_unlock" unlocks "C->waiter_mutex". lib/acl_threadsupport/src/acl_threadsupport.c:301:5: 9.2. destroy: "pthread_mutex_destroy" destroys "C->waiter_mutex". ```
1 parent 9b6385b commit f66baab

File tree

1 file changed

+0
-5
lines changed

1 file changed

+0
-5
lines changed

lib/acl_threadsupport/src/acl_threadsupport.c

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -293,11 +293,6 @@ void acl_reset_condvar(struct acl_condvar_s *C) {
293293
#else
294294
{
295295
int ret = 0;
296-
// Try to unlock then unlock. This is done just in case it is already
297-
// locked, because if the mutex is locked we can't destroy it. This may
298-
// happen in cases where an asssert causes an exit, etc.
299-
pthread_mutex_trylock(&(C->waiter_mutex));
300-
pthread_mutex_unlock(&(C->waiter_mutex));
301296
ret |= pthread_mutex_destroy(&(C->waiter_mutex));
302297
ret |= sem_destroy(&(C->signal_sem));
303298
ret |= sem_destroy(&(C->passive_sem[0]));

0 commit comments

Comments
 (0)