You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2015/03/04 14:22:41 UTC
svn commit: r1663987 - in /subversion/trunk/subversion/libsvn_subr: error.c
pool.c pools.h
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