[svn-commits] tilghman: branch 1.4 r273793 - in /branches/1.4: ./ channels/ include/asterisk/
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Fri Jul 2 16:36:48 CDT 2010
Author: tilghman
Date: Fri Jul 2 16:36:39 2010
New Revision: 273793
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=273793
Log:
Have the DEADLOCK_AVOIDANCE macro warn when an unlock fails, to help catch potentially large software bugs.
(closes issue #17407)
Reported by: pdf
Patches:
20100527__issue17407.diff.txt uploaded by tilghman (license 14)
Review: https://reviewboard.asterisk.org/r/751/
Modified:
branches/1.4/channels/chan_agent.c
branches/1.4/channels/chan_dahdi.c
branches/1.4/channels/chan_h323.c
branches/1.4/channels/chan_local.c
branches/1.4/configure
branches/1.4/configure.ac
branches/1.4/include/asterisk/autoconfig.h.in
branches/1.4/include/asterisk/compiler.h
branches/1.4/include/asterisk/lock.h
Modified: branches/1.4/channels/chan_agent.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/channels/chan_agent.c?view=diff&rev=273793&r1=273792&r2=273793
==============================================================================
--- branches/1.4/channels/chan_agent.c (original)
+++ branches/1.4/channels/chan_agent.c Fri Jul 2 16:36:39 2010
@@ -707,7 +707,12 @@
ast_mutex_lock(&p->lock);
if (p->chan && !ast_check_hangup(p->chan)) {
while (ast_channel_trylock(p->chan)) {
- ast_channel_unlock(ast);
+ int res;
+ if ((res = ast_channel_unlock(ast))) {
+ ast_log(LOG_ERROR, "chan_agent bug! Channel was not locked upon entry to agent_indicate: %s\n", strerror(res));
+ ast_mutex_unlock(&p->lock);
+ return -1;
+ }
usleep(1);
ast_channel_lock(ast);
}
Modified: branches/1.4/channels/chan_dahdi.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/channels/chan_dahdi.c?view=diff&rev=273793&r1=273792&r2=273793
==============================================================================
--- branches/1.4/channels/chan_dahdi.c (original)
+++ branches/1.4/channels/chan_dahdi.c Fri Jul 2 16:36:39 2010
@@ -4399,8 +4399,12 @@
/* Here we have to retain the lock on both the main channel, the 3-way channel, and
the private structure -- not especially easy or clean */
while (p->subs[SUB_THREEWAY].owner && ast_mutex_trylock(&p->subs[SUB_THREEWAY].owner->lock)) {
+ int res;
/* Yuck, didn't get the lock on the 3-way, gotta release everything and re-grab! */
- ast_mutex_unlock(&p->lock);
+ if ((res = ast_mutex_unlock(&p->lock))) {
+ ast_log(LOG_ERROR, "chan_dahdi bug! '&p->lock' was not locked upon entry to 'dahdi_handle_dtmfup': %s\n", strerror(res));
+ return NULL;
+ }
DEADLOCK_AVOIDANCE(&ast->lock);
/* We can grab ast and p in that order, without worry. We should make sure
nothing seriously bad has happened though like some sort of bizarre double
Modified: branches/1.4/channels/chan_h323.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/channels/chan_h323.c?view=diff&rev=273793&r1=273792&r2=273793
==============================================================================
--- branches/1.4/channels/chan_h323.c (original)
+++ branches/1.4/channels/chan_h323.c Fri Jul 2 16:36:39 2010
@@ -313,10 +313,8 @@
if (pvt) {
ast_mutex_lock(&pvt->lock);
/* Don't hold pvt lock while trying to lock the channel */
- while(pvt->owner && ast_channel_trylock(pvt->owner)) {
- ast_mutex_unlock(&pvt->lock);
- usleep(1);
- ast_mutex_lock(&pvt->lock);
+ while (pvt->owner && ast_channel_trylock(pvt->owner)) {
+ DEADLOCK_AVOIDANCE(&pvt->lock);
}
if (pvt->owner) {
Modified: branches/1.4/channels/chan_local.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/channels/chan_local.c?view=diff&rev=273793&r1=273792&r2=273793
==============================================================================
--- branches/1.4/channels/chan_local.c (original)
+++ branches/1.4/channels/chan_local.c Fri Jul 2 16:36:39 2010
@@ -183,10 +183,18 @@
/* Ensure that we have both channels locked */
while (other && ast_channel_trylock(other)) {
- ast_mutex_unlock(&p->lock);
+ int res;
+ if ((res = ast_mutex_unlock(&p->lock))) {
+ ast_log(LOG_ERROR, "chan_local bug! '&p->lock' was not locked when entering local_queue_frame! (%s)\n", strerror(res));
+ return -1;
+ }
if (us && us_locked) {
do {
- ast_channel_unlock(us);
+ if (ast_channel_unlock(us)) {
+ ast_log(LOG_ERROR, "chan_local bug! Our channel was not locked, yet arguments indicated that it was!!\n");
+ ast_mutex_lock(&p->lock);
+ return -1;
+ }
usleep(1);
ast_channel_lock(us);
} while (ast_mutex_trylock(&p->lock));
Modified: branches/1.4/configure.ac
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/configure.ac?view=diff&rev=273793&r1=273792&r2=273793
==============================================================================
--- branches/1.4/configure.ac (original)
+++ branches/1.4/configure.ac Fri Jul 2 16:36:39 2010
@@ -450,6 +450,7 @@
AST_GCC_ATTRIBUTE(unused)
AST_GCC_ATTRIBUTE(always_inline)
AST_GCC_ATTRIBUTE(deprecated)
+AST_GCC_ATTRIBUTE(warn_unused_result)
AC_MSG_CHECKING(for -ffunction-sections support)
saved_CFLAGS="${CFLAGS}"
Modified: branches/1.4/include/asterisk/autoconfig.h.in
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/include/asterisk/autoconfig.h.in?view=diff&rev=273793&r1=273792&r2=273793
==============================================================================
--- branches/1.4/include/asterisk/autoconfig.h.in (original)
+++ branches/1.4/include/asterisk/autoconfig.h.in Fri Jul 2 16:36:39 2010
@@ -66,6 +66,10 @@
/* Define to 1 if your GCC C compiler supports the 'unused' attribute. */
#undef HAVE_ATTRIBUTE_unused
+/* Define to 1 if your GCC C compiler supports the 'warn_unused_result'
+ attribute. */
+#undef HAVE_ATTRIBUTE_warn_unused_result
+
/* Define to 1 if you have the `bzero' function. */
#undef HAVE_BZERO
@@ -467,7 +471,7 @@
/* Define to 1 if you have the `strtoq' function. */
#undef HAVE_STRTOQ
-/* Define to 1 if `st_blksize' is a member of `struct stat'. */
+/* Define to 1 if `st_blksize' is member of `struct stat'. */
#undef HAVE_STRUCT_STAT_ST_BLKSIZE
/* Define to 1 if you have the mISDN Supplemental Services library. */
@@ -656,11 +660,11 @@
/* Define to the one symbol short name of this package. */
#undef PACKAGE_TARNAME
-/* Define to the home page for this package. */
-#undef PACKAGE_URL
-
/* Define to the version of this package. */
#undef PACKAGE_VERSION
+
+/* Define to 1 if the C compiler supports function prototypes. */
+#undef PROTOTYPES
/* Define to necessary symbol if this constant uses a non-standard name on
your system. */
@@ -680,6 +684,11 @@
/* Define to the type of arg 5 for `select'. */
#undef SELECT_TYPE_ARG5
+
+/* Define to 1 if the `setvbuf' function takes the buffering type as its
+ second argument and the buffer pointer as the third, as on System V before
+ release 3. */
+#undef SETVBUF_REVERSED
/* The size of `int', as computed by sizeof. */
#undef SIZEOF_INT
@@ -701,46 +710,50 @@
/* Define to 1 if your <sys/time.h> declares `struct tm'. */
#undef TM_IN_SYS_TIME
-/* Enable extensions on AIX 3, Interix. */
+/* Define to 1 if on AIX 3.
+ System headers sometimes define this.
+ We just want to avoid a redefinition error message. */
#ifndef _ALL_SOURCE
# undef _ALL_SOURCE
#endif
+
+/* Number of bits in a file offset, on hosts where this is settable. */
+#undef _FILE_OFFSET_BITS
+
/* Enable GNU extensions on systems that have them. */
#ifndef _GNU_SOURCE
# undef _GNU_SOURCE
#endif
-/* Enable threading extensions on Solaris. */
+
+/* Define to 1 to make fseeko visible on some hosts (e.g. glibc 2.2). */
+#undef _LARGEFILE_SOURCE
+
+/* Define for large files, on AIX-style hosts. */
+#undef _LARGE_FILES
+
+/* Define to 1 if on MINIX. */
+#undef _MINIX
+
+/* Define to 2 if the system does not provide POSIX.1 features except with
+ this defined. */
+#undef _POSIX_1_SOURCE
+
+/* Define to 1 if you need to in order for `stat' and other things to work. */
+#undef _POSIX_SOURCE
+
+/* Enable extensions on Solaris. */
+#ifndef __EXTENSIONS__
+# undef __EXTENSIONS__
+#endif
#ifndef _POSIX_PTHREAD_SEMANTICS
# undef _POSIX_PTHREAD_SEMANTICS
#endif
-/* Enable extensions on HP NonStop. */
#ifndef _TANDEM_SOURCE
# undef _TANDEM_SOURCE
#endif
-/* Enable general extensions on Solaris. */
-#ifndef __EXTENSIONS__
-# undef __EXTENSIONS__
-#endif
-
-
-/* Number of bits in a file offset, on hosts where this is settable. */
-#undef _FILE_OFFSET_BITS
-
-/* Define to 1 to make fseeko visible on some hosts (e.g. glibc 2.2). */
-#undef _LARGEFILE_SOURCE
-
-/* Define for large files, on AIX-style hosts. */
-#undef _LARGE_FILES
-
-/* Define to 1 if on MINIX. */
-#undef _MINIX
-
-/* Define to 2 if the system does not provide POSIX.1 features except with
- this defined. */
-#undef _POSIX_1_SOURCE
-
-/* Define to 1 if you need to in order for `stat' and other things to work. */
-#undef _POSIX_SOURCE
+
+/* Define like PROTOTYPES; this can be used by system headers. */
+#undef __PROTOTYPES
/* Define to empty if `const' does not conform to ANSI C. */
#undef const
Modified: branches/1.4/include/asterisk/compiler.h
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/include/asterisk/compiler.h?view=diff&rev=273793&r1=273792&r2=273793
==============================================================================
--- branches/1.4/include/asterisk/compiler.h (original)
+++ branches/1.4/include/asterisk/compiler.h Fri Jul 2 16:36:39 2010
@@ -59,4 +59,10 @@
#define attribute_deprecated
#endif
+#ifdef HAVE_ATTRIBUTE_warn_unused_result
+#define attribute_warn_unused_result __attribute__((warn_unused_result))
+#else
+#define attribute_warn_unused_result
+#endif
+
#endif /* _ASTERISK_COMPILER_H */
Modified: branches/1.4/include/asterisk/lock.h
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/include/asterisk/lock.h?view=diff&rev=273793&r1=273792&r2=273793
==============================================================================
--- branches/1.4/include/asterisk/lock.h (original)
+++ branches/1.4/include/asterisk/lock.h Fri Jul 2 16:36:39 2010
@@ -55,6 +55,7 @@
#include "asterisk/time.h"
#endif
#include "asterisk/logger.h"
+#include "asterisk/compiler.h"
/* internal macro to profile mutexes. Only computes the delay on
* non-blocking calls.
@@ -204,13 +205,21 @@
do { \
char __filename[80], __func[80], __mutex_name[80]; \
int __lineno; \
- int __res = ast_find_lock_info(lock, __filename, sizeof(__filename), &__lineno, __func, sizeof(__func), __mutex_name, sizeof(__mutex_name)); \
- ast_mutex_unlock(lock); \
+ int __res2, __res = ast_find_lock_info(lock, __filename, sizeof(__filename), &__lineno, __func, sizeof(__func), __mutex_name, sizeof(__mutex_name)); \
+ __res2 = ast_mutex_unlock(lock); \
usleep(1); \
if (__res < 0) { /* Shouldn't ever happen, but just in case... */ \
- ast_mutex_lock(lock); \
+ if (__res2 == 0) { \
+ ast_mutex_lock(lock); \
+ } else { \
+ ast_log(LOG_WARNING, "Could not unlock mutex '%s': %s and no lock info found! I will NOT try to relock.\n", #lock, strerror(__res2)); \
+ } \
} else { \
- __ast_pthread_mutex_lock(__filename, __lineno, __func, __mutex_name, lock); \
+ if (__res2 == 0) { \
+ __ast_pthread_mutex_lock(__filename, __lineno, __func, __mutex_name, lock); \
+ } else { \
+ ast_log(LOG_WARNING, "Could not unlock mutex '%s': %s. {{{Originally locked at %s line %d: (%s) '%s'}}} I will NOT try to relock.\n", #lock, strerror(__res2), __filename, __lineno, __func, __mutex_name); \
+ } \
} \
} while (0)
@@ -433,6 +442,8 @@
}
static inline int __ast_pthread_mutex_trylock(const char *filename, int lineno, const char *func,
+ const char* mutex_name, ast_mutex_t *t) attribute_warn_unused_result;
+static inline int __ast_pthread_mutex_trylock(const char *filename, int lineno, const char *func,
const char* mutex_name, ast_mutex_t *t)
{
int res;
@@ -712,9 +723,15 @@
#else /* !DEBUG_THREADS */
#define DEADLOCK_AVOIDANCE(lock) \
- ast_mutex_unlock(lock); \
- usleep(1); \
- ast_mutex_lock(lock);
+ do { \
+ int __res; \
+ if (!(__res = ast_mutex_unlock(lock))) { \
+ usleep(1); \
+ ast_mutex_lock(lock); \
+ } else { \
+ ast_log(LOG_WARNING, "Failed to unlock mutex '%s' (%s). I will NOT try to relock. {{{ THIS IS A BUG. }}}\n", #lock, strerror(__res)); \
+ } \
+ } while (0)
typedef pthread_mutex_t ast_mutex_t;
@@ -1367,7 +1384,7 @@
#define ast_channel_lock(x) ast_mutex_lock(&x->lock)
/*! \brief Unlock a channel. If DEBUG_CHANNEL_LOCKS is defined
in the Makefile, print relevant output for debugging */
-#define ast_channel_unlock(x) ast_mutex_unlock(&x->lock)
+#define ast_channel_unlock(x) ast_mutex_unlock(&x->lock)
/*! \brief Try locking a channel. If DEBUG_CHANNEL_LOCKS is defined
in the Makefile, print relevant output for debugging */
#define ast_channel_trylock(x) ast_mutex_trylock(&x->lock)
@@ -1389,7 +1406,7 @@
#define ast_channel_trylock(a) __ast_channel_trylock(a, __FILE__, __LINE__, __PRETTY_FUNCTION__)
/*! \brief Lock AST channel (and print debugging output)
\note You need to enable DEBUG_CHANNEL_LOCKS for this function */
-int __ast_channel_trylock(struct ast_channel *chan, const char *file, int lineno, const char *func);
+int __ast_channel_trylock(struct ast_channel *chan, const char *file, int lineno, const char *func) attribute_warn_unused_result;
#endif
#endif /* _ASTERISK_LOCK_H */
More information about the svn-commits
mailing list