[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: wc-ng locks and pool cleanup

From: Hyrum K. Wright <hyrum_wright_at_mail.utexas.edu>
Date: Tue, 5 Jan 2010 11:30:59 -0600

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 scheme.

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.

-Hyrum
Received on 2010-01-05 18:35:31 CET

This is an archived mail posted to the Subversion Dev mailing list.