So, in an effort to get us past the last of these BDB issues and on to
a final 1.4.0 release I've been staring at the BDB environment code
for the past few hours, trying to figure out what Brane was talking
about with regard to the last remaining pool cleanup ordering issue.
Judging from the comments in his last commit, it's something related
to this section of code in svn_fs_bdb__close:
if (0 == --bdb_baton->error_info->refcount)
{
/* ###
Oh bother, this is another potential pool-cleanup-ordering bug.
Should we just skip this line if bdb->pool==0? See cleanup_env. */
svn_error_clear(bdb_baton->error_info->pending_errors);
#if APR_HAS_THREADS
free(bdb_baton->error_info);
apr_threadkey_private_set(NULL, bdb->error_info);
#endif
}
So clearly he's worried about clearing these errors in some situation
that occurs during global pool cleanup. If you look at cleanup_env,
this is what you find:
static apr_status_t
cleanup_env(void *data)
{
bdb_env_t *bdb = data;
bdb->pool = NULL;
bdb->dbconfig_file = NULL; /* will be closed during pool destruction */
#if APR_HAS_THREADS
apr_threadkey_private_delete(bdb->error_info);
#endif /* APR_HAS_THREADS */
/* If there are no references to this descriptor, free its memory here,
so that we don't leak it if create_env returns an error.
See bdb_close, which takes care of freeing this memory if the
environment is still open when the cache is destroyed. */
if (!bdb->refcount)
free(data);
return APR_SUCCESS;
}
So Brane's comment implies that maybe if bdb->pool is already NULL we
shouldn't bother dealing with bdb->error_info, presumably since it's
already been destroyed via apr_threadkey_private_delete. I'm not
entirely sure this is correct though. We didn't register any
destructor when we created the thread-local variable, so it won't have
actually destroyed the errors. Do we have to put code clearing those
errors into the cleanup_env function, so that if it's called before
the __close code it can clean up the errors, then delete the thread
local var? It'd also have to do the free(), I imagine... The __close
code would then have to check for bdb->pool before it does the clear
and free, right?
Am I on the right track here? Anyone? This stuff is kind of voodoo,
and I'm not all that clear that I'm even close to understanding the
actual problem...
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jun 30 18:36:24 2006