[[[
Plug a memory leak by avoiding repeated failing attempts at apr_xlate setup.
Review by: jerenkrantz
Suggested by: jerenkrantz
dberlin
ghudson
brane
striker
* subversion/libsvn_subr/utf.c
(enotimpl_handle_node): New file-static variable.
(get_xlate_handle_node): Use enotimpl_handle_node to determine
whether to even bother calling apr_xlate_open again. Update and fix
doc string, which was wrong before anyway.
]]]
Index: subversion/libsvn_subr/utf.c
===================================================================
--- subversion/libsvn_subr/utf.c (revision 15373)
+++ subversion/libsvn_subr/utf.c (working copy)
@@ -68,6 +68,17 @@
memory leak. */
static apr_hash_t *xlate_handle_hash = NULL;
+/* Non-NULL iff apr_xlate_open() ever returned APR_ENOTIMPL. Since
+ there would be no point ever calling apr_xlate_open() again in this
+ process, we can save ourself some setup work (including memory
+ allocations) by caching a handle_node to return in that case;
+ enotimpl_handle_node->handle will be NULL, of course.
+ apr_xlate_open() could return APR_ENOTIMPL because APR_HAS_XLATE is
+ not defined, or for some other reason. For our purposes it doesn't
+ matter, we just want to remember that it is not implemented for
+ this process. */
+static xlate_handle_node_t *enotimpl_handle_node = NULL;
+
/* Clean up the xlate handle cache. */
static apr_status_t
xlate_cleanup (void *arg)
@@ -122,12 +133,13 @@
}
}
-/* Return an apr_xlate handle for converting from FROMPAGE to
- TOPAGE. Create one if it doesn't exist in USERDATA_KEY. If
- unable to find a handle, or unable to create one because
- apr_xlate_open returned APR_EINVAL, then set *RET to null and
- return SVN_NO_ERROR; if fail for some other reason, return
- error. */
+/* Set *RET to an apr_xlate handle for converting from FROMPAGE to
+ TOPAGE, creating the handle if it doesn't exist in USERDATA_KEY.
+ If unable to find a handle, or unable to create one because
+ apr_xlate_open returned APR_EINVAL or APR_ENOTIMPL, then
+ *RET->handle will be NULL (and in the ENOTIMPL case, allocate *RET
+ itself in a child of the top-level pool, not in POOL). If fail for
+ any other reason, return error. */
static svn_error_t *
get_xlate_handle_node (xlate_handle_node_t **ret,
const char *topage, const char *frompage,
@@ -199,6 +211,14 @@
if (userdata_key && xlate_handle_hash)
pool = apr_hash_pool_get (xlate_handle_hash);
+ /* If we failed due to ENOTIMPL before, then we'd certainly fail
+ now, so just return without trying. */
+ if (enotimpl_handle_node)
+ {
+ *ret = enotimpl_handle_node;
+ return SVN_NO_ERROR;
+ }
+
/* Try to create a handle. */
*ret = apr_palloc (pool, sizeof(xlate_handle_node_t));
apr_err = apr_xlate_open (&(*ret)->handle, topage, frompage, pool);
@@ -229,6 +249,19 @@
if (APR_STATUS_IS_EINVAL (apr_err) || APR_STATUS_IS_ENOTIMPL (apr_err))
{
+ if (APR_STATUS_IS_ENOTIMPL (apr_err))
+ {
+ /* Remember for next time that we needn't bother trying.
+ Use the top-level pool because we have no lifetime
+ guarantees about our local pool parameter. */
+ enotimpl_handle_node = apr_palloc (svn_pool_create (NULL),
+ sizeof (*enotimpl_handle_node));
+ enotimpl_handle_node->handle = NULL;
+ enotimpl_handle_node->frompage = NULL;
+ enotimpl_handle_node->topage = NULL;
+ enotimpl_handle_node->next = NULL;
+ }
+
(*ret)->handle = NULL;
return SVN_NO_ERROR;
}