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
