You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2014/05/06 00:14:36 UTC

svn commit: r1592640 - in /subversion/trunk/subversion: include/private/svn_mutex.h libsvn_subr/mutex.c

Author: stefan2
Date: Mon May  5 22:14:36 2014
New Revision: 1592640

URL: http://svn.apache.org/r1592640
Log:
Follow-up to r1591919:  Make recursive lock attempts also detectable
on systems without thread support.  On those, we will simply count
the lock attempts (counter must never exceed 1).  This fixes the
test failures on our BSD build bots.

* subversion/include/private/svn_mutex.h
  (svn_mutex__t): Always have a mutex struct.

* subversion/libsvn_subr/mutex.c
  (svn_mutex__t): On non-threaded systems, the struct contains a
                  simple lock counter.
  (svn_mutex__init): Always allocate and initialize a mutex structure.
  (svn_mutex__lock,
   svn_mutex__unlock): On non-threaded systems, use simple ref counting
                       to detect recursive locking attempts.

Modified:
    subversion/trunk/subversion/include/private/svn_mutex.h
    subversion/trunk/subversion/libsvn_subr/mutex.c

Modified: subversion/trunk/subversion/include/private/svn_mutex.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_mutex.h?rev=1592640&r1=1592639&r2=1592640&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_mutex.h (original)
+++ subversion/trunk/subversion/include/private/svn_mutex.h Mon May  5 22:14:36 2014
@@ -39,7 +39,6 @@ extern "C" {
  * This is a simple wrapper around @c apr_thread_mutex_t and will be a
  * valid identifier even if APR does not support threading.
  */
-#if APR_HAS_THREADS
 
 /** A mutex for synchronization between threads. It may be NULL, in
  * which case no synchronization will take place. The latter is useful
@@ -47,14 +46,6 @@ extern "C" {
  */
 typedef struct svn_mutex__t svn_mutex__t;
 
-#else
-
-/** Dummy definition. The content will never be actually accessed.
- */
-typedef void svn_mutex__t;
-
-#endif
-
 /** Initialize the @a *mutex. If @a mutex_required is TRUE, the mutex will
  * actually be created with a lifetime defined by @a result_pool. Otherwise,
  * the pointer will be set to @c NULL and svn_mutex__lock() as well as

Modified: subversion/trunk/subversion/libsvn_subr/mutex.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/mutex.c?rev=1592640&r1=1592639&r2=1592640&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/mutex.c (original)
+++ subversion/trunk/subversion/libsvn_subr/mutex.c Mon May  5 22:14:36 2014
@@ -27,8 +27,6 @@
 #include "private/svn_atomic.h"
 #include "private/svn_mutex.h"
 
-#if APR_HAS_THREADS
-
 /* With CHECKED set to TRUE, LOCKED and OWNER must be set *after* acquiring
  * the MUTEX and be reset *before* releasing it again.  This is sufficient
  * because we only want to check whether the current thread already holds
@@ -37,11 +35,13 @@
  */
 struct svn_mutex__t
 {
-  apr_thread_mutex_t *mutex;
-
   /* If TRUE, perform extra checks to detect attempts at recursive locking. */
   svn_boolean_t checked;
 
+#if APR_HAS_THREADS
+
+  apr_thread_mutex_t *mutex;
+
   /* The owner of this lock, if locked or NULL, otherwise.  Since NULL might
    * be a valid owner ID on some systems, checking for NULL may not be 100%
    * accurate.  Be sure to only produce false negatives in that case.
@@ -52,9 +52,16 @@ struct svn_mutex__t
    * setting and resetting it is never racy (but reading it may be).
    * Only used when CHECKED is set. */
   volatile void *owner;
-};
+
+#else
+
+  /* If there is no multi-threading support, simply count lock attempts. */
+  int count;
+
 #endif
 
+};
+
 svn_error_t *
 svn_mutex__init(svn_mutex__t **mutex_p,
                 svn_boolean_t mutex_required,
@@ -65,21 +72,22 @@ svn_mutex__init(svn_mutex__t **mutex_p,
      strictly necessary if APR_HAS_THREADS has not been set */
   *mutex_p = NULL;
 
-#if APR_HAS_THREADS
   if (mutex_required)
     {
       svn_mutex__t *mutex = apr_pcalloc(result_pool, sizeof(*mutex));
+
+#if APR_HAS_THREADS
       apr_status_t status =
           apr_thread_mutex_create(&mutex->mutex,
                                   APR_THREAD_MUTEX_DEFAULT,
                                   result_pool);
       if (status)
         return svn_error_wrap_apr(status, _("Can't create mutex"));
+#endif
 
       mutex->checked = checked;
       *mutex_p = mutex;
     }
-#endif
 
   return SVN_NO_ERROR;
 }
@@ -87,9 +95,9 @@ svn_mutex__init(svn_mutex__t **mutex_p,
 svn_error_t *
 svn_mutex__lock(svn_mutex__t *mutex)
 {
-#if APR_HAS_THREADS
   if (mutex)
     {
+#if APR_HAS_THREADS
       apr_status_t status;
       void *current_thread;
       void *lock_owner;
@@ -136,8 +144,17 @@ svn_mutex__lock(svn_mutex__t *mutex)
           /* Set "us" as the new owner. */
           apr_atomic_casptr(&mutex->owner, current_thread, NULL);
         }
-    }
+#else
+      if (mutex->checked)
+        {
+          /* We want non-threaded systems to detect the same coding errors
+             as threaded systems. */
+          if (++mutex->count > 1)
+            return svn_error_create(SVN_ERR_RECURSIVE_LOCK, NULL, 
+                                    _("Recursive locks are not supported"));
+        }
 #endif
+    }
 
   return SVN_NO_ERROR;
 }
@@ -146,9 +163,9 @@ svn_error_t *
 svn_mutex__unlock(svn_mutex__t *mutex,
                   svn_error_t *err)
 {
-#if APR_HAS_THREADS
   if (mutex)
     {
+#if APR_HAS_THREADS
       apr_status_t status;
 
       /* We will soon no longer be the owner of this lock.  So, reset the
@@ -172,8 +189,12 @@ svn_mutex__unlock(svn_mutex__t *mutex,
       status = apr_thread_mutex_unlock(mutex->mutex);
       if (status && !err)
         return svn_error_wrap_apr(status, _("Can't unlock mutex"));
-    }
+#else
+      /* Update lock counter. */
+      if (mutex->checked)
+        --mutex->count;
 #endif
+    }
 
   return err;
 }