subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Hyrum K. Wright" <>
Subject Re: wc-ng locks and pool cleanup
Date Tue, 05 Jan 2010 17:30:59 GMT

On Dec 21, 2009, at 11:16 AM, Philip Martin wrote:

> In 1.6 wc locks were associated with access batons and when an access
> baton was opened a pool cleanup handler was installed to ensure that
> the baton was closed.  The pool cleanup handler would remove any lock
> associated with the access baton provided there were no log files in
> the directory.  Code was typically:
>    svn_wc_adm_access_t *adm_access;
>    SVN_ERR(svn_wc_adm_open3(&adm_access, ... pool));
>    SVN_ERR(svn_wc_foo(adm_access, ... pool));
>    SVN_ERR(svn_wc_adm_close(adm_access));
> If svn_wc_foo completes without error then svn_wc_adm_close is called
> which removes the locks unconditionally.  If svn_wc_foo returns an
> error then svn_wc_adm_close doesn't get called, and when the pool is
> finally cleared the pool cleanup handler will leave or remove the lock
> depending on whether cleanup is required.
> The locks in wc-ng don't have the same behaviour, they only get
> removed by an explict svn_wc__release_write_lock call.  If the code
> above is simply converted to wc-ng code then an error from svn_wc_foo
> will leave the working copy locked; that's what happens when I try to
> convert libsvn_client/copy.c.  The client code can be modifed to work
> around this in various ways but I don't think it's a good idea to
> require the client code to do this.
> I think svn_wc__acquire_write_lock should associate the lock with a
> pool and install a pool cleanup handler that removes the lock if the
> workqueue is empty.  I don't think it makes sense to use the db
> context state_pool since applications may want to reuse the context
> for multiple operations; the pool lifetime is too long.  I think it
> should be a pool passed in to svn_wc__acquire_write_lock so that the
> caller can control the lifetime of the lock.

I like the idea but wonder what impact (if any) would the use of inherited locks have this

Relatedly, I'm having a spot of bother with a patch of my own and the use of pools to cleanup
locks.  In trying to switch to inherited locks, I've ran into a scenario where the sqlite
db for a lock is being closed before are done using it, causing problems when we later attempt
to look up information in that database.

View raw message