From 754b9cc24ed886c955b5df5a12217e1539df99f4 Mon Sep 17 00:00:00 2001 From: Christian Thaeter Date: Sun, 2 Sep 2007 00:52:40 +0200 Subject: [PATCH 1/6] C wraper for condition variables --- src/lib/Makefile.am | 7 +- src/lib/locking.c | 52 ++++++++++ src/lib/locking.h | 226 ++++++++++++++++++++++++++++++++++++++++++ tests/15locking.tests | 8 ++ tests/Makefile.am | 6 +- 5 files changed, 296 insertions(+), 3 deletions(-) create mode 100644 src/lib/locking.c create mode 100644 src/lib/locking.h create mode 100644 tests/15locking.tests diff --git a/src/lib/Makefile.am b/src/lib/Makefile.am index 91327cdf5..305f06ee9 100644 --- a/src/lib/Makefile.am +++ b/src/lib/Makefile.am @@ -25,11 +25,14 @@ libcin3_a_SOURCES = \ $(libcin3_a_srcdir)/plugin.c \ $(libcin3_a_srcdir)/error.c \ $(libcin3_a_srcdir)/time.c \ - $(libcin3_a_srcdir)/framerate.c + $(libcin3_a_srcdir)/framerate.c \ + $(libcin3_a_srcdir)/locking.c + noinst_HEADERS += \ $(libcin3_a_srcdir)/plugin.h \ $(libcin3_a_srcdir)/error.h \ $(libcin3_a_srcdir)/time.h \ - $(libcin3_a_srcdir)/framerate.h + $(libcin3_a_srcdir)/framerate.h \ + $(libcin3_a_srcdir)/locking.h diff --git a/src/lib/locking.c b/src/lib/locking.c new file mode 100644 index 000000000..744a083e8 --- /dev/null +++ b/src/lib/locking.c @@ -0,0 +1,52 @@ +/* + locking.c - locking primitives + + Copyright (C) CinelerraCV + 2007, Christian Thaeter + + This program is free software; you can redistribute it and/or + modify it under the terms of the GNU General Public License as + published by the Free Software Foundation; either version 2 of the + License, or (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. +*/ + +#include "lib/locking.h" + +CinelerraCondition +cinelerra_condition_init (CinelerraCondition self) +{ + if (self) + { + pthread_cond_init (&self->cond, NULL); + pthread_mutex_init (&self->mutex, NULL); + } + return self; +} + +CinelerraCondition +cinelerra_condition_destroy (CinelerraCondition self) +{ + if (self) + { + if (pthread_mutex_destroy (&self->mutex)) + CINELERRA_DIE; + else if (pthread_cond_destroy (&self->cond)) + CINELERRA_DIE; + else + return self; + } + return NULL; +} + + + + diff --git a/src/lib/locking.h b/src/lib/locking.h new file mode 100644 index 000000000..067dee822 --- /dev/null +++ b/src/lib/locking.h @@ -0,0 +1,226 @@ +/* + locking.h - locking primitives + + Copyright (C) CinelerraCV + 2007, Christian Thaeter + + This program is free software; you can redistribute it and/or + modify it under the terms of the GNU General Public License as + published by the Free Software Foundation; either version 2 of the + License, or (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. +*/ + +#ifndef CINELERRA_LOCKING_H +#define CINELERRA_LOCKING_H + +#include +#include + +#include "lib/error.h" + +/** + * used to store the current lock state. + * + * + */ +enum cinelerra_lockstate + { + CINELERRA_UNLOCKED, + CINELERRA_LOCKED + }; + + +/** + * Condition variables. + * + */ +struct cinelerra_condition_struct +{ + pthread_cond_t cond; + pthread_mutex_t mutex; +}; +typedef struct cinelerra_condition_struct cinelerra_condition; +typedef cinelerra_condition* CinelerraCondition; + + +/** + * Initialize a condition variable + * @param self is a pointer to the condition variable to be initialized + * @return self as given + */ +CinelerraCondition +cinelerra_condition_init (CinelerraCondition self); + + +/** + * destroy a condition variable + * @param self is a pointer to the condition variable to be initialized + * @return self on success or NULL at error + */ +CinelerraCondition +cinelerra_condition_destroy (CinelerraCondition self); + + +/** + * signal a single waiting thread. + * @param self condition variable to be signaled, must be given, all errors are fatal + */ +static inline void +cinelerra_condition_signal (CinelerraCondition self) +{ + REQUIRE (self); + if (pthread_mutex_lock (&self->mutex)) + CINELERRA_DIE; + pthread_cond_signal (&self->cond); + if (pthread_mutex_unlock (&self->mutex)) + CINELERRA_DIE; +} + +/** + * signal all waiting threads + * @param self condition variable to be signaled, must be given, all errors are fatal + */ +static inline void +cinelerra_condition_broadcast (CinelerraCondition self) +{ + REQUIRE (self); + if (pthread_mutex_lock (&self->mutex)) + CINELERRA_DIE; + pthread_cond_broadcast (&self->cond); + if (pthread_mutex_unlock (&self->mutex)) + CINELERRA_DIE; +} + + + + + +/** + * conditionlock used to manage the state of a condition variable. + */ +struct cinelerra_conditionlock_struct +{ + CinelerraCondition cond; + enum cinelerra_lockstate state; +}; +typedef struct cinelerra_conditionlock_struct cinelerra_conditionlock; +typedef struct cinelerra_conditionlock_struct* CinelerraConditionlock; + +/* helper function for nobug */ +static inline void +cinelerra_conditionlock_ensureunlocked (CinelerraConditionlock self) +{ + ENSURE (self->state == CINELERRA_UNLOCKED, "forgot to unlock the condition mutex"); +} + +/* override with a macro to use the cleanup checker */ +#define cinelerra_conditionlock \ +cinelerra_conditionlock NOBUG_CLEANUP(cinelerra_conditionlock_ensureunlocked) + + +/** + * initialize a conditionlock state + * @param self conditionlock to be initialized, must be an automatic variable + * @param cond associated condition variable + * @param state initial state of the mutex, either CINELERRA_LOCKED or CINELERRA_UNLOCKED + * @return self as given + * errors are fatal + */ +static inline CinelerraConditionlock +cinelerra_conditionlock_init (CinelerraConditionlock self, CinelerraCondition cond, enum cinelerra_lockstate state) +{ + REQUIRE (self); + REQUIRE (cond); + self->cond = cond; + self->state = state; + if (state == CINELERRA_LOCKED) + if (pthread_mutex_lock (&cond->mutex)) + CINELERRA_DIE; + + return self; +} + +/** + * lock the mutex. + * must not already be locked + * @param self conditionlock associated with a condition variable + */ +static inline void +cinelerra_conditionlock_lock (CinelerraConditionlock self) +{ + REQUIRE (self); + REQUIRE (self->state == CINELERRA_UNLOCKED, "mutex already locked"); + + if (pthread_mutex_lock (&self->cond->mutex)) + CINELERRA_DIE; + + self->state = CINELERRA_LOCKED; +} + + +/** + * wait on a locked condition. + * Waits until the condition variable gets signaled from another thread. Must already be locked. + * @param self conditionlock associated with a condition variable + */ +static inline void +cinelerra_conditionlock_wait (CinelerraConditionlock self) +{ + REQUIRE (self); + REQUIRE (self->state == CINELERRA_LOCKED, "mutex must be locked"); + pthread_cond_wait (&self->cond->cond, &self->cond->mutex); +} + + +/** + * release mutex. + * a conditionlock must be unlocked before leaving scope + * @param self conditionlock associated with a condition variable + */ +static inline int +cinelerra_conditionlock_unlock (CinelerraConditionlock self) +{ + REQUIRE (self); + REQUIRE (self->state == CINELERRA_LOCKED, "mutex was not locked"); + if (pthread_mutex_unlock (&self->cond->mutex)) + CINELERRA_DIE; + self->state = CINELERRA_UNLOCKED; +} + + +/** + * signal a single waiting thread + * @param self conditionlock associated with the condition variable to be signaled + */ +static inline void +cinelerra_conditionlock_signal (CinelerraConditionlock self) +{ + REQUIRE (self); + REQUIRE (self->state == CINELERRA_LOCKED, "mutex was not locked"); + pthread_cond_signal (&self->cond->cond); +} + + +/** + * signal all waiting threads + * @param self conditionlock associated with the condition variable to be signaled + */ +static inline int +cinelerra_conditionlock_broadcast (CinelerraConditionlock self) +{ + REQUIRE (self); + REQUIRE (self->state == CINELERRA_LOCKED, "mutex was not locked"); + pthread_cond_broadcast (&self->cond->cond); +} + + +#endif diff --git a/tests/15locking.tests b/tests/15locking.tests new file mode 100644 index 000000000..1ef4938c5 --- /dev/null +++ b/tests/15locking.tests @@ -0,0 +1,8 @@ + +TESTING "Locking" ./test-locking + +TEST "condition not unlocked asserts" conditionforgotunlock < Date: Sun, 2 Sep 2007 13:53:33 +0200 Subject: [PATCH 2/6] removed tests/.gitignore please run tests in $builddir (the patterns ignored sourcefiles) --- tests/.gitignore | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 tests/.gitignore diff --git a/tests/.gitignore b/tests/.gitignore deleted file mode 100644 index 0ae34206f..000000000 --- a/tests/.gitignore +++ /dev/null @@ -1,5 +0,0 @@ -,* -test-* -mainsuite -errortest -plugin-example From 0d1097315c92025a0870e1ea2a8637de0628e35c Mon Sep 17 00:00:00 2001 From: Christian Thaeter Date: Sun, 2 Sep 2007 13:56:33 +0200 Subject: [PATCH 3/6] renamed locking.* to condition.* --- src/lib/Makefile.am | 4 +- src/lib/{locking.c => condition.c} | 4 +- src/lib/{locking.h => condition.h} | 12 +++--- tests/Makefile.am | 8 ++-- tests/locking/test-condition.c | 65 ++++++++++++++++++++++++++++++ 5 files changed, 79 insertions(+), 14 deletions(-) rename src/lib/{locking.c => condition.c} (95%) rename src/lib/{locking.h => condition.h} (96%) create mode 100644 tests/locking/test-condition.c diff --git a/src/lib/Makefile.am b/src/lib/Makefile.am index 305f06ee9..0256ca2ea 100644 --- a/src/lib/Makefile.am +++ b/src/lib/Makefile.am @@ -26,7 +26,7 @@ libcin3_a_SOURCES = \ $(libcin3_a_srcdir)/error.c \ $(libcin3_a_srcdir)/time.c \ $(libcin3_a_srcdir)/framerate.c \ - $(libcin3_a_srcdir)/locking.c + $(libcin3_a_srcdir)/condition.c noinst_HEADERS += \ @@ -34,5 +34,5 @@ noinst_HEADERS += \ $(libcin3_a_srcdir)/error.h \ $(libcin3_a_srcdir)/time.h \ $(libcin3_a_srcdir)/framerate.h \ - $(libcin3_a_srcdir)/locking.h + $(libcin3_a_srcdir)/condition.h diff --git a/src/lib/locking.c b/src/lib/condition.c similarity index 95% rename from src/lib/locking.c rename to src/lib/condition.c index 744a083e8..c5bd19a6c 100644 --- a/src/lib/locking.c +++ b/src/lib/condition.c @@ -1,5 +1,5 @@ /* - locking.c - locking primitives + condition.c - condition variable Copyright (C) CinelerraCV 2007, Christian Thaeter @@ -19,7 +19,7 @@ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ -#include "lib/locking.h" +#include "lib/condition.h" CinelerraCondition cinelerra_condition_init (CinelerraCondition self) diff --git a/src/lib/locking.h b/src/lib/condition.h similarity index 96% rename from src/lib/locking.h rename to src/lib/condition.h index 067dee822..60714ed9f 100644 --- a/src/lib/locking.h +++ b/src/lib/condition.h @@ -1,5 +1,5 @@ /* - locking.h - locking primitives + condition.h - condition variables Copyright (C) CinelerraCV 2007, Christian Thaeter @@ -19,8 +19,8 @@ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ -#ifndef CINELERRA_LOCKING_H -#define CINELERRA_LOCKING_H +#ifndef CINELERRA_CONDITION_H +#define CINELERRA_CONDITION_H #include #include @@ -32,7 +32,7 @@ * * */ -enum cinelerra_lockstate +enum cinelerra_conditionstate { CINELERRA_UNLOCKED, CINELERRA_LOCKED @@ -110,7 +110,7 @@ cinelerra_condition_broadcast (CinelerraCondition self) struct cinelerra_conditionlock_struct { CinelerraCondition cond; - enum cinelerra_lockstate state; + enum cinelerra_conditionstate state; }; typedef struct cinelerra_conditionlock_struct cinelerra_conditionlock; typedef struct cinelerra_conditionlock_struct* CinelerraConditionlock; @@ -136,7 +136,7 @@ cinelerra_conditionlock NOBUG_CLEANUP(cinelerra_conditionlock_ensureunlocked) * errors are fatal */ static inline CinelerraConditionlock -cinelerra_conditionlock_init (CinelerraConditionlock self, CinelerraCondition cond, enum cinelerra_lockstate state) +cinelerra_conditionlock_init (CinelerraConditionlock self, CinelerraCondition cond, enum cinelerra_conditionstate state) { REQUIRE (self); REQUIRE (cond); diff --git a/tests/Makefile.am b/tests/Makefile.am index 96c58df11..403f9d065 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -18,7 +18,7 @@ tests_srcdir = $(top_srcdir)/tests -check_PROGRAMS += test-error test-time test-locking +check_PROGRAMS += test-error test-time test-condition test_error_SOURCES = $(tests_srcdir)/error/errortest.c test_error_CPPFLAGS = $(AM_CPPFLAGS) -std=gnu99 -Wall -Werror -I$(top_srcdir)/src/ @@ -28,8 +28,8 @@ test_time_SOURCES = $(tests_srcdir)/time/test-time.c test_time_CPPFLAGS = $(AM_CPPFLAGS) -std=gnu99 -Wall -Werror -I$(top_srcdir)/src/ test_time_LDADD = $(builddir)/libcin3.a -lnobugmt -lpthread -ldl -lm -test_locking_SOURCES = $(tests_srcdir)/locking/test-locking.c -test_locking_CPPFLAGS = $(AM_CPPFLAGS) -std=gnu99 -Wall -Werror -I$(top_srcdir)/src/ -test_locking_LDADD = $(builddir)/libcin3.a -lnobugmt -lpthread -ldl -lm +test_condition_SOURCES = $(tests_srcdir)/locking/test-condition.c +test_condition_CPPFLAGS = $(AM_CPPFLAGS) -std=gnu99 -Wall -Werror -I$(top_srcdir)/src/ +test_condition_LDADD = $(builddir)/libcin3.a -lnobugmt -lpthread -ldl -lm TESTS = $(tests_srcdir)/test.sh diff --git a/tests/locking/test-condition.c b/tests/locking/test-condition.c new file mode 100644 index 000000000..6f0f32365 --- /dev/null +++ b/tests/locking/test-condition.c @@ -0,0 +1,65 @@ +/* + test-locking.c - test locking functions + + Copyright (C) CinelerraCV + 2007, Christian Thaeter + + This program is free software; you can redistribute it and/or + modify it under the terms of the GNU General Public License as + published by the Free Software Foundation; either version 2 of the + License, or (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. +*/ + +#include +#include + +#include "lib/condition.h" + +CINELERRA_ERROR_DEFINE(TEST, "test error"); + +#if 0 +waiting_thread() +{ + lock; + wait; + unlock; +} + + +signaling_thread() +{ + signal(); +} +#endif + + +int +main (int argc, char** argv) +{ + NOBUG_INIT; + + if (argc == 1) + return 0; + + if (!strcmp(argv[1], "conditionforgotunlock")) + { + cinelerra_condition c; + cinelerra_condition_init (&c); + + cinelerra_conditionlock NOBUG_CLEANUP(cinelerra_conditionlock_ensureunlocked) l; + cinelerra_conditionlock_init (&l, &c, CINELERRA_LOCKED); + } + else + return 1; + + return 0; +} From dd67216f388aab86e9b05b059ed56ce3dc32ef72 Mon Sep 17 00:00:00 2001 From: Christian Thaeter Date: Sun, 2 Sep 2007 14:54:25 +0200 Subject: [PATCH 4/6] notes about locking primitives --- wiki/support_library.html | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/wiki/support_library.html b/wiki/support_library.html index ac753035f..efc4009dc 100644 --- a/wiki/support_library.html +++ b/wiki/support_library.html @@ -1134,6 +1134,34 @@ config.formatters.push( { } ) //}}} +
+
! Provided Locking Primitives
+The support library provides wrappers around some pthread locking primitives to make their usage more robust and easier.
+
+The basic concept is that each locking primitive is an object as well as each locker is implemented as object too, this adds a small convenience layer for robustness. We use ~NoBug to assert that locks are properly unlocked.
+
+!! Mutex
+We only support fast (non recursive, non errorcheck) mutexes for now. Debugging deadlock detection will be done in ~NoBug. If we need dynamic deadlock detection we will have to support errorcheck mutexes at demand. Same for recursive mutexes.
+
+!! Condition Variables
+Condition variables are simple synchronization devices, refer to the doxygen docs about using them. One needs to lock them when preparing to wait on them and finally unlock them. While signaling can optionally be done without a locker object (locking is implicit then).
+
+!! Read/Write Locks
+Similar to mutexes we support multiple-reader/one-writer locks, they can be used whenever many concurrent read accesses and rare write accesses are expected to some datastructure (profile this later). When congestion rather unexpected then prefer a mutex.
+
+! No Semaphores Rationale
+Semaphores have quite ugly semantics and are very hard to debug. For now (and likely forever) we will not to use them.
+
+! ~ToDo
+!! trylock and timedlock
+.. is not yet implemented but will be added in request.
+
+!! Barriers
+... will be added on request too.
+
+!! Thread Cancellation
+Thread cancellation policies are not yet finally decided, for now we consider threads uncancelable!
+
''[[Cinelerra3|index.html]]''
 SupportLibrary
@@ -2391,7 +2419,7 @@ h1,h2,h3,h4,h5,h6 {
 /*}}}*/
-
+
The Support Library contains all tools we need at various places, but by themselves don't defines a subsystem on their own.
 
 These things are:
@@ -2399,7 +2427,7 @@ These things are:
 * [[ErrorHandling]]
 * a wrapper for POSIX Threads
 ** Thread creation joining and canceling
-** Locking primitives like Condition variables and Mutexes
+** [[Locking primitives like Condition variables and Mutexes|LockingPrimitives]]
 * [[Frame and Time handling and calculations|FrameAndTime]]
  
 (... to be continued)

From 7445c1798d267cea66411453ea07a80f94cd9c31 Mon Sep 17 00:00:00 2001
From: Christian Thaeter 
Date: Sun, 2 Sep 2007 16:19:50 +0200
Subject: [PATCH 5/6] generic locking.h for shared declarations

---
 src/lib/Makefile.am |  1 +
 src/lib/condition.h | 20 +++-----------------
 src/lib/locking.h   | 43 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 17 deletions(-)
 create mode 100644 src/lib/locking.h

diff --git a/src/lib/Makefile.am b/src/lib/Makefile.am
index 0256ca2ea..320a579c2 100644
--- a/src/lib/Makefile.am
+++ b/src/lib/Makefile.am
@@ -34,5 +34,6 @@ noinst_HEADERS += \
 	$(libcin3_a_srcdir)/error.h \
 	$(libcin3_a_srcdir)/time.h \
 	$(libcin3_a_srcdir)/framerate.h \
+	$(libcin3_a_srcdir)/locking.h \
 	$(libcin3_a_srcdir)/condition.h
 
diff --git a/src/lib/condition.h b/src/lib/condition.h
index 60714ed9f..7efad439c 100644
--- a/src/lib/condition.h
+++ b/src/lib/condition.h
@@ -22,21 +22,7 @@
 #ifndef CINELERRA_CONDITION_H
 #define CINELERRA_CONDITION_H
 
-#include 
-#include 
-
-#include "lib/error.h"
-
-/**
- * used to store the current lock state.
- *
- *
- */
-enum cinelerra_conditionstate
-  {
-    CINELERRA_UNLOCKED,
-    CINELERRA_LOCKED
-  };
+#include "lib/locking.h"
 
 
 /**
@@ -110,7 +96,7 @@ cinelerra_condition_broadcast (CinelerraCondition self)
 struct cinelerra_conditionlock_struct
 {
   CinelerraCondition cond;
-  enum cinelerra_conditionstate  state;
+  enum cinelerra_lockstate  state;
 };
 typedef struct cinelerra_conditionlock_struct cinelerra_conditionlock;
 typedef struct cinelerra_conditionlock_struct* CinelerraConditionlock;
@@ -136,7 +122,7 @@ cinelerra_conditionlock NOBUG_CLEANUP(cinelerra_conditionlock_ensureunlocked)
  * errors are fatal
  */
 static inline CinelerraConditionlock
-cinelerra_conditionlock_init (CinelerraConditionlock self, CinelerraCondition cond, enum cinelerra_conditionstate state)
+cinelerra_conditionlock_init (CinelerraConditionlock self, CinelerraCondition cond, enum cinelerra_lockstate state)
 {
   REQUIRE (self);
   REQUIRE (cond);
diff --git a/src/lib/locking.h b/src/lib/locking.h
new file mode 100644
index 000000000..aa722a568
--- /dev/null
+++ b/src/lib/locking.h
@@ -0,0 +1,43 @@
+/*
+  locking.h  -  shared declarations for all locking primitives
+
+  Copyright (C)         CinelerraCV
+    2007,               Christian Thaeter 
+
+  This program is free software; you can redistribute it and/or
+  modify it under the terms of the GNU General Public License as
+  published by the Free Software Foundation; either version 2 of the
+  License, or (at your option) any later version.
+
+  This program is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+  GNU General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; if not, write to the Free Software
+  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+*/
+
+#ifndef CINELERRA_LOCKING_H
+#define CINELERRA_LOCKING_H
+
+#include 
+#include 
+
+#include "lib/error.h"
+
+/**
+ * used to store the current lock state.
+ *
+ *
+ */
+enum cinelerra_lockstate
+  {
+    CINELERRA_UNLOCKED,
+    CINELERRA_LOCKED,
+    CINELERRA_RLOCKED,
+    CINELERRA_WLOCKED
+  };
+
+#endif

From d50ab9fe2125f985eb4ae9a610f4944cddc425bd Mon Sep 17 00:00:00 2001
From: Christian Thaeter 
Date: Sun, 2 Sep 2007 17:51:55 +0200
Subject: [PATCH 6/6] cosmetic fixes

---
 src/lib/condition.c | 4 +---
 src/lib/condition.h | 6 +++---
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/lib/condition.c b/src/lib/condition.c
index c5bd19a6c..0d731f959 100644
--- a/src/lib/condition.c
+++ b/src/lib/condition.c
@@ -41,10 +41,8 @@ cinelerra_condition_destroy (CinelerraCondition self)
         CINELERRA_DIE;
       else if (pthread_cond_destroy (&self->cond))
         CINELERRA_DIE;
-      else
-        return self;
     }
-  return NULL;
+  return self;
 }
 
 
diff --git a/src/lib/condition.h b/src/lib/condition.h
index 7efad439c..e55c8ed0b 100644
--- a/src/lib/condition.h
+++ b/src/lib/condition.h
@@ -48,9 +48,9 @@ cinelerra_condition_init (CinelerraCondition self);
 
 
 /**
- * destroy a condition variable
- * @param self is a pointer to the condition variable to be initialized
- * @return self on success or NULL at error
+ * Destroy a condition variable
+ * @param self is a pointer to the condition variable to be destroyed
+ * @return self as given
  */
 CinelerraCondition
 cinelerra_condition_destroy (CinelerraCondition self);