Various fixes to r10788, r10796. * include/svn_utf.h (svn_utf_initialize): Add pool argument. * libsvn_subr/utf.c (svn_utf_initialize): Create the hashtable pool as a subpool of the pool passed in the new argument. (get_xlate_handle_node): Look for the xlate handle in the pool if there is no global cache. Register a cleanup that marks the node as defunct. Check for defunct nodes before returning them. (put_xlate_handle_node): New pool argument, callers updated. Store the handle in the pool if there is no global cache. Store the handle in the global cache, if one, even in the single-threaded case. * libsvn_subr/cmdline.c (svn_cmdline_init): Don't initialize the UTF-8 routines here, since we don't have a pool available. * svnadmin/main.c, svnlook/main.c, clients/cmdline/main.x, svndumpfilter/main.c, svnversion/main.c (main): Initialize the UTF-8 routines. Index: subversion/svnadmin/main.c =================================================================== --- subversion/svnadmin/main.c (revision 10850) +++ subversion/svnadmin/main.c (arbetskopia) @@ -960,6 +960,9 @@ return EXIT_FAILURE; } + /* Initialize the UTF-8 routines. */ + svn_utf_initialize (pool); + if (argc <= 1) { subcommand_help (NULL, NULL, pool); Index: subversion/include/svn_utf.h =================================================================== --- subversion/include/svn_utf.h (revision 10850) +++ subversion/include/svn_utf.h (arbetskopia) @@ -36,12 +36,13 @@ /** @since New in 1.1. * Initialize the UTF-8 encoding/decoding routines. + * Allocate cached translation handles in a subpool of @a pool. * NOTE: It is optional to call this function, but if it is used, no other * svn function may be in use in other threads during the call of this - * function. + * function or when @a pool is cleared or destroyed. * Initializing the UTF-8 routines will improve performance. */ -void svn_utf_initialize (void); +void svn_utf_initialize (apr_pool_t *pool); /** Set @a *dest to a utf8-encoded stringbuf from native stringbuf @a src; * allocate @a *dest in @a pool. Index: subversion/libsvn_subr/utf.c =================================================================== --- subversion/libsvn_subr/utf.c (revision 10850) +++ subversion/libsvn_subr/utf.c (arbetskopia) @@ -72,29 +72,38 @@ return APR_SUCCESS; } +/* Set the handle of ARG to NULL. */ +static apr_status_t +xlate_handle_node_cleanup (void *arg) +{ + xlate_handle_node_t *node = arg; + + node->handle = NULL; + return APR_SUCCESS; +} + void -svn_utf_initialize (void) +svn_utf_initialize (apr_pool_t *pool) { - apr_pool_t *pool; + apr_pool_t *subpool; #if APR_HAS_THREADS apr_thread_mutex_t *mutex; #endif if (!xlate_handle_hash) { - /* We create our own pool, which we protect with the mutex. - It will be destroyed during APR termination. */ - pool = svn_pool_create (NULL); + /* We create our own subpool, which we protect with the mutex. */ + subpool = svn_pool_create (pool); #if APR_HAS_THREADS - if (apr_thread_mutex_create (&mutex, APR_THREAD_MUTEX_DEFAULT, pool) + if (apr_thread_mutex_create (&mutex, APR_THREAD_MUTEX_DEFAULT, subpool) == APR_SUCCESS) xlate_handle_mutex = mutex; else return; #endif - xlate_handle_hash = apr_hash_make (pool); - apr_pool_cleanup_register (pool, NULL, xlate_cleanup, + xlate_handle_hash = apr_hash_make (subpool); + apr_pool_cleanup_register (subpool, NULL, xlate_cleanup, apr_pool_cleanup_null); } } @@ -110,15 +119,15 @@ const char *topage, const char *frompage, const char *userdata_key, apr_pool_t *pool) { - xlate_handle_node_t *old_handle = NULL; + void *old_handle = NULL; apr_status_t apr_err; /* If we already have a handle, just return it. */ if (userdata_key) { + if (xlate_handle_hash) + { #if APR_HAS_THREADS - if (xlate_handle_mutex) - { apr_err = apr_thread_mutex_lock (xlate_handle_mutex); if (apr_err != APR_SUCCESS) return svn_error_create (apr_err, NULL, @@ -128,48 +137,71 @@ APR_HASH_KEY_STRING); if (old_handle) { - /* Remove from the hash table. */ - apr_hash_set (xlate_handle_hash, userdata_key, - APR_HASH_KEY_STRING, old_handle->next); - old_handle->next = NULL; - apr_err = apr_thread_mutex_unlock (xlate_handle_mutex); - if (apr_err != APR_SUCCESS) - return svn_error_create (apr_err, NULL, - "Can't unlock charset " - "translation mutex"); *ret = old_handle; - return SVN_NO_ERROR; + /* Ensure that the handle is still valid. */ + if ((*ret)->handle) + { + /* Remove from the hash table. */ + apr_hash_set (xlate_handle_hash, userdata_key, + APR_HASH_KEY_STRING, (*ret)->next); + (*ret)->next = NULL; + apr_err = apr_thread_mutex_unlock (xlate_handle_mutex); + if (apr_err != APR_SUCCESS) + return svn_error_create (apr_err, NULL, + "Can't unlock charset " + "translation mutex"); + return SVN_NO_ERROR; + } } - } #else /* ! APR_HAS_THREADS */ - if (xlate_handle_hash) - { old_handle = apr_hash_get(xlate_handle_hash, userdata_key, APR_HASH_KEY_STRING); if (old_handle) { *ret = old_handle; - return SVN_NO_ERROR; + /* Ensure that the handle is still valid. */ + if ((*ret)->handle) + return SVN_NO_ERROR; } +#endif } -#endif + else + { + /* We fall back on a per-pool cache instead. */ + apr_pool_userdata_get (&old_handle, userdata_key, pool); + if (old_handle) + { + *ret = old_handle; + /* Ensure that the handle is still valid. */ + if ((*ret)->handle) + return SVN_NO_ERROR; + } + } } - /* Note that we still have the mutex locked, so we can use the global - pool for creating the new xlate handle. */ + /* Note that we still have the mutex locked (if it is initialized), so we + can use the global pool for creating the new xlate handle. */ /* Use the correct pool for creating the handle. */ if (userdata_key && xlate_handle_hash) pool = apr_hash_pool_get (xlate_handle_hash); - /* Try to create one. */ + /* Try to create a handle. */ *ret = apr_palloc (pool, sizeof(xlate_handle_node_t)); apr_err = apr_xlate_open (&(**ret).handle, topage, frompage, pool); (**ret).next = NULL; + /* If we are called from inside a pool cleanup handler, the just created + xlate handle will be closed when that handler returns by a newly + registered cleanup handler, however, the handle is still cached by us. + To prevent this, we register a cleanup handler that will reset our + handle, so we don't use an invalid one. */ + apr_pool_cleanup_register (pool, *ret, xlate_handle_node_cleanup, + apr_pool_cleanup_null); + /* Don't need the lock anymore. */ #if APR_HAS_THREADS - if (xlate_handle_mutex) + if (xlate_handle_hash) { apr_status_t unlock_err = apr_thread_mutex_unlock (xlate_handle_mutex); if (unlock_err != APR_SUCCESS) @@ -196,29 +228,38 @@ } /* Put back NODE into the xlate handle cache for use by other calls. + If there is no global cache, store the handle in POOL. Ignore errors related to locking/unlocking the mutex. ### Mutex errors here are very weird. Should we handle them "correctly" ### even if that complicates error handling in the routines below? */ static void put_xlate_handle_node (xlate_handle_node_t *node, - const char *userdata_key) + const char *userdata_key, + apr_pool_t *pool) { assert (node->next == NULL); if (!userdata_key) return; + if (xlate_handle_hash) + { #if APR_HAS_THREADS - if (xlate_handle_mutex) - { if (apr_thread_mutex_lock (xlate_handle_mutex) != APR_SUCCESS) abort (); +#endif node->next = apr_hash_get (xlate_handle_hash, userdata_key, APR_HASH_KEY_STRING); apr_hash_set (xlate_handle_hash, userdata_key, APR_HASH_KEY_STRING, node); +#if APR_HAS_THREADS if (apr_thread_mutex_unlock (xlate_handle_mutex) != APR_SUCCESS) abort (); +#endif } -#endif + else + { + /* Store it in the per-pool cache. */ + apr_pool_userdata_set (node, userdata_key, apr_pool_cleanup_null, pool); + } } /* Return the apr_xlate handle for converting native characters to UTF-8. */ @@ -435,7 +476,7 @@ { err = convert_to_stringbuf (node->handle, src->data, src->len, dest, pool); - put_xlate_handle_node (node, SVN_UTF_NTOU_XLATE_HANDLE); + put_xlate_handle_node (node, SVN_UTF_NTOU_XLATE_HANDLE, pool); SVN_ERR (err); return check_utf8 ((*dest)->data, (*dest)->len, pool); } @@ -463,7 +504,7 @@ { err = convert_to_stringbuf (node->handle, src->data, src->len, &destbuf, pool); - put_xlate_handle_node (node, SVN_UTF_NTOU_XLATE_HANDLE); + put_xlate_handle_node (node, SVN_UTF_NTOU_XLATE_HANDLE, pool); SVN_ERR (err); SVN_ERR (check_utf8 (destbuf->data, destbuf->len, pool)); *dest = svn_string_create_from_buf (destbuf, pool); @@ -515,7 +556,7 @@ SVN_ERR (get_ntou_xlate_handle_node (&node, pool)); err = convert_cstring (dest, src, node->handle, pool); - put_xlate_handle_node (node, SVN_UTF_NTOU_XLATE_HANDLE); + put_xlate_handle_node (node, SVN_UTF_NTOU_XLATE_HANDLE, pool); SVN_ERR (err); SVN_ERR (check_cstring_utf8 (*dest, pool)); @@ -535,7 +576,7 @@ SVN_ERR (get_xlate_handle_node (&node, "UTF-8", frompage, convset_key, pool)); err = convert_cstring (dest, src, node->handle, pool); - put_xlate_handle_node (node, convset_key); + put_xlate_handle_node (node, convset_key, pool); SVN_ERR (err); SVN_ERR (check_cstring_utf8 (*dest, pool)); @@ -557,7 +598,7 @@ { SVN_ERR (check_utf8 (src->data, src->len, pool)); err = convert_to_stringbuf (node->handle, src->data, src->len, dest, pool); - put_xlate_handle_node (node, SVN_UTF_UTON_XLATE_HANDLE); + put_xlate_handle_node (node, SVN_UTF_UTON_XLATE_HANDLE, pool); return err; } else @@ -585,7 +626,7 @@ SVN_ERR (check_utf8 (src->data, src->len, pool)); err = convert_to_stringbuf (node->handle, src->data, src->len, &dbuf, pool); - put_xlate_handle_node (node, SVN_UTF_UTON_XLATE_HANDLE); + put_xlate_handle_node (node, SVN_UTF_UTON_XLATE_HANDLE, pool); SVN_ERR (err); *dest = svn_string_create_from_buf (dbuf, pool); } @@ -610,7 +651,7 @@ SVN_ERR (get_uton_xlate_handle_node (&node, pool)); SVN_ERR (check_utf8 (src, strlen (src), pool)); err = convert_cstring (dest, src, node->handle, pool); - put_xlate_handle_node (node, SVN_UTF_UTON_XLATE_HANDLE); + put_xlate_handle_node (node, SVN_UTF_UTON_XLATE_HANDLE, pool); SVN_ERR (err); return SVN_NO_ERROR; @@ -630,7 +671,7 @@ SVN_ERR (get_xlate_handle_node (&node, topage, "UTF-8", convset_key, pool)); SVN_ERR (check_utf8 (src, strlen (src), pool)); err = convert_cstring (dest, src, node->handle, pool); - put_xlate_handle_node (node, convset_key); + put_xlate_handle_node (node, convset_key, pool); return err; } @@ -741,7 +782,7 @@ SVN_ERR (check_utf8 (src->data, src->len, pool)); err = convert_to_stringbuf (node->handle, src->data, src->len, &dbuf, pool); - put_xlate_handle_node (node, SVN_UTF_UTON_XLATE_HANDLE); + put_xlate_handle_node (node, SVN_UTF_UTON_XLATE_HANDLE, pool); SVN_ERR (err); *dest = dbuf->data; return SVN_NO_ERROR; Index: subversion/libsvn_subr/cmdline.c =================================================================== --- subversion/libsvn_subr/cmdline.c (revision 10850) +++ subversion/libsvn_subr/cmdline.c (arbetskopia) @@ -152,9 +152,6 @@ return EXIT_FAILURE; } - /* Initialize the UTF-8 subsystem. */ - svn_utf_initialize (); - #ifdef ENABLE_NLS #ifdef WIN32 { Index: subversion/svnlook/main.c =================================================================== --- subversion/svnlook/main.c (revision 10850) +++ subversion/svnlook/main.c (arbetskopia) @@ -1832,6 +1832,9 @@ return EXIT_FAILURE; } + /* Initialize the UTF-8 routines. */ + svn_utf_initialize (pool); + if (argc <= 1) { subcommand_help (NULL, NULL, pool); Index: subversion/clients/cmdline/main.c =================================================================== --- subversion/clients/cmdline/main.c (revision 10850) +++ subversion/clients/cmdline/main.c (arbetskopia) @@ -775,6 +775,8 @@ if (err) return error_exit (err, stderr, FALSE, pool); + /* INitialize the UTF-8 routines. */ + svn_utf_initialize (pool); /* Begin processing arguments. */ opt_state.start_revision.kind = svn_opt_revision_unspecified; opt_state.end_revision.kind = svn_opt_revision_unspecified; Index: subversion/svndumpfilter/main.c =================================================================== --- subversion/svndumpfilter/main.c (revision 10850) +++ subversion/svndumpfilter/main.c (arbetskopia) @@ -1022,6 +1022,9 @@ return EXIT_FAILURE; } + /* Initialize the UTF-8 routines. */ + svn_utf_initialize (pool); + if (argc <= 1) { subcommand_help (NULL, NULL, pool); Index: subversion/svnversion/main.c =================================================================== --- subversion/svnversion/main.c (revision 10850) +++ subversion/svnversion/main.c (arbetskopia) @@ -227,6 +227,9 @@ return EXIT_FAILURE; } + /* Initialize the UTF-8 routines. */ + svn_utf_initialize (pool); + sb.switched = FALSE; sb.modified = FALSE; sb.committed = FALSE;