apr-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From s.@apache.org
Subject svn commit: r1593614 - in /apr/apr/trunk: CHANGES configure.in memory/unix/apr_pools.c
Date Fri, 09 May 2014 20:14:01 GMT
Author: sf
Date: Fri May  9 20:14:01 2014
New Revision: 1593614

URL: http://svn.apache.org/r1593614
Log:
Option to detect concurrent accesses to pools

Enabled by new configure option --enable-pool-concurrency-check.

Compared to pool-owner-debugging, this only detects cases where there is actual
contention between accesses. The advantage is that runtime costs should be
relatively low.

The diagnostic messages could still use some improvement.


Modified:
    apr/apr/trunk/CHANGES
    apr/apr/trunk/configure.in
    apr/apr/trunk/memory/unix/apr_pools.c

Modified: apr/apr/trunk/CHANGES
URL: http://svn.apache.org/viewvc/apr/apr/trunk/CHANGES?rev=1593614&r1=1593613&r2=1593614&view=diff
==============================================================================
--- apr/apr/trunk/CHANGES [utf-8] (original)
+++ apr/apr/trunk/CHANGES [utf-8] Fri May  9 20:14:01 2014
@@ -1,6 +1,10 @@
                                                      -*- coding: utf-8 -*-
 Changes for APR 2.0.0
 
+  *) Add new --enable-pool-concurrency-check configure option to detect
+     thread-unsafe concurrent accesses to pools. Runtime costs should be
+     relatively low. [Stefan Fritsch]
+
   *) Add apr_sockaddr_info_copy(), for making a deep copy of an
      apr_sockaddr_t into a specified pool.  [Yann Ylavic
      <ylavic.dev gmail.com>]

Modified: apr/apr/trunk/configure.in
URL: http://svn.apache.org/viewvc/apr/apr/trunk/configure.in?rev=1593614&r1=1593613&r2=1593614&view=diff
==============================================================================
--- apr/apr/trunk/configure.in (original)
+++ apr/apr/trunk/configure.in Fri May  9 20:14:01 2014
@@ -1492,6 +1492,14 @@ AC_ARG_ENABLE(allocator-uses-mmap,
     fi ]
 )
 
+AC_ARG_ENABLE(pool-concurrency-check,
+  [  --enable-pool-concurrency-check Check for concurrent usage of memory pools],
+  [ if test "$enableval" = "yes"; then
+    AC_DEFINE(APR_POOL_CONCURRENCY_CHECK, 1,
+               [Define if pool functions should abort if concurrent usage is detected])
+    fi ]
+)
+
 dnl ----------------------------- Checks for standard typedefs
 AC_TYPE_OFF_T
 AC_TYPE_PID_T

Modified: apr/apr/trunk/memory/unix/apr_pools.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/memory/unix/apr_pools.c?rev=1593614&r1=1593613&r2=1593614&view=diff
==============================================================================
--- apr/apr/trunk/memory/unix/apr_pools.c (original)
+++ apr/apr/trunk/memory/unix/apr_pools.c Fri May  9 20:14:01 2014
@@ -49,6 +49,11 @@
 int apr_running_on_valgrind = 0;
 #endif
 
+#if APR_POOL_CONCURRENCY_CHECK && !APR_HAS_THREADS
+#error pool-concurrency-check does not make sense without threads
+#endif
+
+
 /*
  * Magic numbers
  */
@@ -540,6 +545,10 @@ struct apr_pool_t {
     apr_os_proc_t         owner_proc;
 #endif /* defined(NETWARE) */
     cleanup_t            *pre_cleanups;
+#if APR_POOL_CONCURRENCY_CHECK
+    volatile apr_uint32_t in_use;
+    apr_os_thread_t       in_use_by;
+#endif /* APR_POOL_CONCURRENCY_CHECK */
 };
 
 #define SIZEOF_POOL_T       APR_ALIGN_DEFAULT(sizeof(apr_pool_t))
@@ -673,6 +682,65 @@ APR_DECLARE(void) apr_pool_terminate(voi
 #define node_free_space(node_) ((apr_size_t)(node_->endp - node_->first_avail))
 
 /*
+ * Helpers to mark pool as in-use/free. Used for finding thread-unsafe
+ * concurrent accesses from different threads.
+ */
+#if APR_POOL_CONCURRENCY_CHECK
+static void pool_concurrency_abort(apr_pool_t *pool, const char *func, apr_uint32_t old)
+{
+    fprintf(stderr, "%s: previous state %d in_use_by/cur %lx/%lx pool %p(%s)\n",
+            func, old,
+            (unsigned long)pool->in_use_by,
+            (unsigned long)apr_os_thread_current(),
+            pool, pool->tag);
+    abort();
+}
+
+static APR_INLINE void pool_concurrency_set_used(apr_pool_t *pool)
+{
+    apr_uint32_t old;
+
+    old = apr_atomic_cas32(&pool->in_use, 1, 0);
+
+    if (old == 2 && pool->in_use_by == apr_os_thread_current()) {
+        /* cleanups may use the pool */
+        return;
+    }
+
+    if (old != 0)
+        pool_concurrency_abort(pool, __func__, old);
+
+    pool->in_use_by = apr_os_thread_current();
+}
+
+static APR_INLINE void pool_concurrency_set_idle(apr_pool_t *pool)
+{
+    apr_uint32_t old;
+
+    old = apr_atomic_cas32(&pool->in_use, 0, 1);
+
+    if (old != 1)
+        pool_concurrency_abort(pool, __func__, old);
+}
+
+static APR_INLINE void pool_concurrency_init(apr_pool_t *pool)
+{
+    pool->in_use = 0;
+}
+
+static APR_INLINE void pool_concurrency_set_destroyed(apr_pool_t *pool)
+{
+    apr_atomic_set32(&pool->in_use, 3);
+    pool->in_use_by = apr_os_thread_current();
+}
+#else
+static APR_INLINE void pool_concurrency_init(apr_pool_t *pool)          { }
+static APR_INLINE void pool_concurrency_set_used(apr_pool_t *pool)      { }
+static APR_INLINE void pool_concurrency_set_idle(apr_pool_t *pool)      { }
+static APR_INLINE void pool_concurrency_set_destroyed(apr_pool_t *pool) { }
+#endif /* APR_POOL_CONCURRENCY_CHECK */
+
+/*
  * Memory allocation
  */
 
@@ -682,12 +750,14 @@ APR_DECLARE(void *) apr_palloc(apr_pool_
     void *mem;
     apr_size_t size, free_index;
 
+    pool_concurrency_set_used(pool);
     size = APR_ALIGN_DEFAULT(in_size);
 #if HAVE_VALGRIND
     if (apr_running_on_valgrind)
         size += 2 * REDZONE;
 #endif
     if (size < in_size) {
+        pool_concurrency_set_idle(pool);
         if (pool->abort_fn)
             pool->abort_fn(APR_ENOMEM);
 
@@ -708,6 +778,7 @@ APR_DECLARE(void *) apr_palloc(apr_pool_
     }
     else {
         if ((node = allocator_alloc(pool->allocator, size)) == NULL) {
+            pool_concurrency_set_idle(pool);
             if (pool->abort_fn)
                 pool->abort_fn(APR_ENOMEM);
 
@@ -743,14 +814,17 @@ APR_DECLARE(void *) apr_palloc(apr_pool_
 have_mem:
 #if HAVE_VALGRIND
     if (!apr_running_on_valgrind) {
+        pool_concurrency_set_idle(pool);
         return mem;
     }
     else {
         mem = (char *)mem + REDZONE;
         VALGRIND_MEMPOOL_ALLOC(pool, mem, in_size);
+        pool_concurrency_set_idle(pool);
         return mem;
     }
 #else
+    pool_concurrency_set_idle(pool);
     return mem;
 #endif
 }
@@ -786,7 +860,10 @@ APR_DECLARE(void) apr_pool_clear(apr_poo
 
     /* Run pre destroy cleanups */
     run_cleanups(&pool->pre_cleanups);
+
+    pool_concurrency_set_used(pool);
     pool->pre_cleanups = NULL;
+    pool_concurrency_set_idle(pool);
 
     /* Destroy the subpools.  The subpools will detach themselves from
      * this pool thus this loop is safe and easy.
@@ -796,6 +873,8 @@ APR_DECLARE(void) apr_pool_clear(apr_poo
 
     /* Run cleanups */
     run_cleanups(&pool->cleanups);
+
+    pool_concurrency_set_used(pool);
     pool->cleanups = NULL;
     pool->free_cleanups = NULL;
 
@@ -814,13 +893,17 @@ APR_DECLARE(void) apr_pool_clear(apr_poo
 
     APR_IF_VALGRIND(VALGRIND_MEMPOOL_TRIM(pool, pool, 1));
 
-    if (active->next == active)
+    if (active->next == active) {
+        pool_concurrency_set_idle(pool);
         return;
+    }
 
     *active->ref = NULL;
     allocator_free(pool->allocator, active->next);
     active->next = active;
     active->ref = &active->next;
+
+    pool_concurrency_set_idle(pool);
 }
 
 APR_DECLARE(void) apr_pool_destroy(apr_pool_t *pool)
@@ -830,7 +913,10 @@ APR_DECLARE(void) apr_pool_destroy(apr_p
 
     /* Run pre destroy cleanups */
     run_cleanups(&pool->pre_cleanups);
+
+    pool_concurrency_set_used(pool);
     pool->pre_cleanups = NULL;
+    pool_concurrency_set_idle(pool);
 
     /* Destroy the subpools.  The subpools will detach themselve from
      * this pool thus this loop is safe and easy.
@@ -840,6 +926,7 @@ APR_DECLARE(void) apr_pool_destroy(apr_p
 
     /* Run cleanups */
     run_cleanups(&pool->cleanups);
+    pool_concurrency_set_destroyed(pool);
 
     /* Free subprocesses */
     free_proc_chain(pool->subprocesses);
@@ -985,6 +1072,8 @@ APR_DECLARE(apr_status_t) apr_pool_creat
         pool->ref = NULL;
     }
 
+    pool_concurrency_init(pool);
+
     *newpool = pool;
 
     return APR_SUCCESS;
@@ -1050,6 +1139,8 @@ APR_DECLARE(apr_status_t) apr_pool_creat
 #endif /* defined(NETWARE) */
     if (!allocator)
         pool_allocator->owner = pool;
+
+    pool_concurrency_init(pool);
     *newpool = pool;
 
     return APR_SUCCESS;
@@ -1195,6 +1286,7 @@ APR_DECLARE(char *) apr_pvsprintf(apr_po
     apr_memnode_t *active, *node;
     apr_size_t free_index;
 
+    pool_concurrency_set_used(pool);
     ps.node = active = pool->active;
     ps.pool = pool;
     ps.vbuff.curpos  = ps.node->first_avail;
@@ -1257,8 +1349,10 @@ APR_DECLARE(char *) apr_pvsprintf(apr_po
     /*
      * Link the node in if it's a new one
      */
-    if (!ps.got_a_new_node)
+    if (!ps.got_a_new_node) {
+        pool_concurrency_set_idle(pool);
         return strp;
+    }
 
     active = pool->active;
     node = ps.node;
@@ -1275,8 +1369,10 @@ APR_DECLARE(char *) apr_pvsprintf(apr_po
     active->free_index = free_index;
     node = active->next;
 
-    if (free_index >= node->free_index)
+    if (free_index >= node->free_index) {
+        pool_concurrency_set_idle(pool);
         return strp;
+    }
 
     do {
         node = node->next;
@@ -1286,9 +1382,11 @@ APR_DECLARE(char *) apr_pvsprintf(apr_po
     list_remove(active);
     list_insert(active, node);
 
+    pool_concurrency_set_idle(pool);
     return strp;
 
 error:
+    pool_concurrency_set_idle(pool);
     if (pool->abort_fn)
         pool->abort_fn(APR_ENOMEM);
     if (ps.got_a_new_node) {



Mime
View raw message