subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From br...@apache.org
Subject svn commit: r1663987 - in /subversion/trunk/subversion/libsvn_subr: error.c pool.c pools.h
Date Wed, 04 Mar 2015 13:22:41 GMT
Author: brane
Date: Wed Mar  4 13:22:41 2015
New Revision: 1663987

URL: http://svn.apache.org/r1663987
Log:
Make the error location tracking thread-safe.

* subversion/libsvn_subr/pools.h: New file.
  (svn_pool__create_unmanaged): New prototype.
* subversion/libsvn_subr/pool.c
  (svn_pool__create_unmanaged): Implement.

* subversion/libsvn_subr/error.c:
   Conditionally include apr_thread_proc.h, private/svn_atomic.h and pools.h.
  (error_file_key, error_line_key,
   null_threadkey_dtor, locate_init_once): New; available in debug mode
   if APR supports threads.
  (svn_error__locate, make_error_internal): Use thread-local storage
   when available for the error location info.

Added:
    subversion/trunk/subversion/libsvn_subr/pools.h   (with props)
Modified:
    subversion/trunk/subversion/libsvn_subr/error.c
    subversion/trunk/subversion/libsvn_subr/pool.c

Modified: subversion/trunk/subversion/libsvn_subr/error.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/error.c?rev=1663987&r1=1663986&r2=1663987&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/error.c (original)
+++ subversion/trunk/subversion/libsvn_subr/error.c Wed Mar  4 13:22:41 2015
@@ -28,6 +28,10 @@
 #include <apr_pools.h>
 #include <apr_strings.h>
 
+#if defined(SVN_DEBUG) && APR_HAS_THREADS
+#include <apr_thread_proc.h>
+#endif
+
 #include <zlib.h>
 
 #ifndef SVN_ERR__TRACING
@@ -38,12 +42,56 @@
 #include "svn_pools.h"
 #include "svn_utf.h"
 
+#include "private/svn_error_private.h"
+#include "svn_private_config.h"
+
+#if defined(SVN_DEBUG) && APR_HAS_THREADS
+#include "private/svn_atomic.h"
+#include "pools.h"
+#endif
+
+
 #ifdef SVN_DEBUG
-/* XXX FIXME: These should be protected by a thread mutex.
-   svn_error__locate and make_error_internal should cooperate
-   in locking and unlocking it. */
+#  if APR_HAS_THREADS
+static apr_threadkey_t *error_file_key = NULL;
+static apr_threadkey_t *error_line_key = NULL;
+
+/* No-op destructor for apr_threadkey_private_create(). */
+static void null_threadkey_dtor(void *stuff) {}
+
+/* Handler for svn_atomic__init_once used by svn_error__locate to
+   initialize the thread-local error location storage.
+   This function will never return an error. */
+static svn_error_t *
+locate_init_once(void *ignored_baton, apr_pool_t *ignored_pool)
+{
+  /* Strictly speaking, this is a memory leak, since we're creating an
+     unmanaged, top-level pool and never destroying it.  We do this
+     because this pool controls the lifetime of the thread-local
+     storage for error locations, and that storage must always be
+     available. */
+  apr_pool_t *threadkey_pool = svn_pool__create_unmanaged(TRUE);
+  apr_status_t status;
+
+  status = apr_threadkey_private_create(&error_file_key,
+                                        null_threadkey_dtor,
+                                        threadkey_pool);
+  if (status == APR_SUCCESS)
+    status = apr_threadkey_private_create(&error_line_key,
+                                          null_threadkey_dtor,
+                                          threadkey_pool);
+
+  /* If anything went wrong with the creation of the thread-local
+     storage, we'll revert to the old, thread-agnostic behaviour */
+  if (status != APR_SUCCESS)
+    error_file_key = error_line_key = NULL;
+
+  return SVN_NO_ERROR;
+}
+#  endif  /* APR_HAS_THREADS */
 
-/* XXX TODO: Define mutex here #if APR_HAS_THREADS */
+/* These location variables will be used in no-threads mode or if
+   thread-local storage is not available. */
 static const char * volatile error_file = NULL;
 static long volatile error_line = -1;
 
@@ -51,9 +99,6 @@ static long volatile error_line = -1;
 static const char SVN_FILE_LINE_UNDEFINED[] = "svn:<undefined>";
 #endif /* SVN_DEBUG */
 
-#include "svn_private_config.h"
-#include "private/svn_error_private.h"
-
 
 /*
  * Undefine the helpers for creating errors.
@@ -76,11 +121,27 @@ static const char SVN_FILE_LINE_UNDEFINE
 void
 svn_error__locate(const char *file, long line)
 {
-#if defined(SVN_DEBUG)
-  /* XXX TODO: Lock mutex here */
+#ifdef SVN_DEBUG
+#  if APR_HAS_THREADS
+  static volatile svn_atomic_t init_status = 0;
+  svn_error_clear(svn_atomic__init_once(&init_status,
+                                        locate_init_once,
+                                        NULL, NULL));
+
+  if (error_file_key && error_line_key)
+    {
+      apr_status_t status;
+      status = apr_threadkey_private_set((char*)file, error_file_key);
+      if (status == APR_SUCCESS)
+        status = apr_threadkey_private_set((void*)line, error_line_key);
+      if (status == APR_SUCCESS)
+        return;
+    }
+#  endif  /* APR_HAS_THREADS */
+
   error_file = file;
   error_line = line;
-#endif
+#endif /* SVN_DEBUG */
 }
 
 
@@ -103,6 +164,7 @@ make_error_internal(apr_status_t apr_err
 {
   apr_pool_t *pool;
   svn_error_t *new_error;
+  apr_status_t status = APR_SUCCESS;
 
   /* Reuse the child's pool, or create our own. */
   if (child)
@@ -121,16 +183,34 @@ make_error_internal(apr_status_t apr_err
   new_error->apr_err = apr_err;
   new_error->child   = child;
   new_error->pool    = pool;
-#if defined(SVN_DEBUG)
-  new_error->file    = error_file;
-  new_error->line    = error_line;
-  /* XXX TODO: Unlock mutex here */
+
+#ifdef SVN_DEBUG
+#if APR_HAS_THREADS
+  if (error_file_key && error_line_key)
+    {
+      void *item;
+      status = apr_threadkey_private_get(&item, error_file_key);
+      if (status == APR_SUCCESS)
+        {
+          new_error->file = item;
+          status = apr_threadkey_private_get(&item, error_line_key);
+          if (status == APR_SUCCESS)
+            new_error->line = (long)item;
+        }
+    }
+#  endif  /* APR_HAS_THREADS */
+
+  if (status != APR_SUCCESS)
+    {
+      new_error->file = error_file;
+      new_error->line = error_line;
+    }
 
   if (! child)
       apr_pool_cleanup_register(pool, new_error,
                                 err_abort,
                                 apr_pool_cleanup_null);
-#endif
+#endif /* SVN_DEBUG */
 
   return new_error;
 }

Modified: subversion/trunk/subversion/libsvn_subr/pool.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/pool.c?rev=1663987&r1=1663986&r2=1663987&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/pool.c (original)
+++ subversion/trunk/subversion/libsvn_subr/pool.c Wed Mar  4 13:22:41 2015
@@ -26,12 +26,15 @@
 #include <stdlib.h>
 #include <stdio.h>
 
+#include <apr.h>
+#include <apr_version.h>
 #include <apr_general.h>
 #include <apr_pools.h>
 #include <apr_thread_mutex.h>
 
 #include "svn_pools.h"
 
+#include "pools.h"
 
 #if APR_POOL_DEBUG
 /* file_line for the non-debug case. */
@@ -140,3 +143,24 @@ svn_pool_create_allocator(svn_boolean_t
 
   return allocator;
 }
+
+
+/*
+ * apr_pool_create_core_ex was introduced in APR 1.3.0, then
+ * deprecated and renamed to apr_pool_create_unmanaged_ex in 1.3.3.
+ * Since our minimum requirement is APR 1.3.0, one or the other of
+ * these functions will always be available.
+ */
+#if !APR_VERSION_AT_LEAST(1,3,3)
+#define apr_pool_create_unmanaged_ex apr_pool_create_core_ex
+#endif
+
+/* Private function that creates an unmanaged pool. */
+apr_pool_t *
+svn_pool__create_unmanaged(svn_boolean_t thread_safe)
+{
+  apr_pool_t *pool;
+  apr_pool_create_unmanaged_ex(&pool, abort_on_pool_failure,
+                               svn_pool_create_allocator(thread_safe));
+  return pool;
+}

Added: subversion/trunk/subversion/libsvn_subr/pools.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/pools.h?rev=1663987&view=auto
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/pools.h (added)
+++ subversion/trunk/subversion/libsvn_subr/pools.h Wed Mar  4 13:22:41 2015
@@ -0,0 +1,43 @@
+/*
+ * pools.h: private pool functions
+ *
+ * ====================================================================
+ *    Licensed to the Apache Software Foundation (ASF) under one
+ *    or more contributor license agreements.  See the NOTICE file
+ *    distributed with this work for additional information
+ *    regarding copyright ownership.  The ASF licenses this file
+ *    to you under the Apache License, Version 2.0 (the
+ *    "License"); you may not use this file except in compliance
+ *    with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing,
+ *    software distributed under the License is distributed on an
+ *    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *    KIND, either express or implied.  See the License for the
+ *    specific language governing permissions and limitations
+ *    under the License.
+ * ====================================================================
+ */
+
+#ifndef SVN_LIBSVN_SUBR_POOLS_H
+#define SVN_LIBSVN_SUBR_POOLS_H
+
+#include "svn_pools.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif /* __cplusplus */
+
+/* Create an unmanaged, global pool with a new allocator.
+   THREAD_SAFE indicates whether the pool's allocator should be
+   thread-safe or not. */
+apr_pool_t *
+svn_pool__create_unmanaged(svn_boolean_t thread_safe);
+
+#ifdef __cplusplus
+}
+#endif /* __cplusplus */
+
+#endif /* SVN_LIBSVN_SUBR_POOLS_H */

Propchange: subversion/trunk/subversion/libsvn_subr/pools.h
------------------------------------------------------------------------------
    svn:eol-style = native



Mime
View raw message