Philip Martin <philip.mar...@wandisco.com> writes:
> hwri...@apache.org writes:
>
>> Author: hwright
>> Date: Thu Nov 19 18:09:08 2009
>> New Revision: 882232
>
>> --- subversion/trunk/subversion/libsvn_client/revert.c (original)
>> +++ subversion/trunk/subversion/libsvn_client/revert.c Thu Nov 19 18:09:08
>> 2009
>> @@ -68,19 +68,13 @@
>> svn_client_ctx_t *ctx,
>> apr_pool_t *pool)
>> {
>> - svn_wc_adm_access_t *adm_access, *target_access;
>> - const char *target, *local_abspath;
>> + const char *local_abspath;
>> svn_error_t *err;
>> - int adm_lock_level = SVN_WC__LEVELS_TO_LOCK_FROM_DEPTH(depth);
>> -
>> - SVN_ERR(svn_wc__adm_open_anchor_in_context(
>> - &adm_access, &target_access, &target,
>> - ctx->wc_ctx, path, TRUE, adm_lock_level,
>> - ctx->cancel_func, ctx->cancel_baton,
>> - pool));
>>
>> SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
>>
>> + SVN_ERR(svn_wc__acquire_write_lock(ctx->wc_ctx, local_abspath, pool));
>> +
>> err = svn_wc_revert4(ctx->wc_ctx,
>> local_abspath,
>> depth,
>> @@ -108,7 +102,8 @@
>> return svn_error_return(err);
>> }
>>
>> - return svn_wc_adm_close2(adm_access, pool);
>> + return svn_error_return(
>> + svn_wc__release_write_lock(ctx->wc_ctx, local_abspath, pool));
>> }
>
> Your change conflicts with my change shown below. The problem is
> shown by depth_tests 33 which reverts a schedule add directory. Your
> code above assumes after calling svn_wc_revert4 on such a directory it
> is still possible to call svn_wc__release_write_lock. Is that valid
> given that revert makes the whole directory vanish?
So there might be a problem in my code, but I still don't understand
your code. The problem is when local_abspath is a schedule add
directory. Your code acquires a write lock on local_abspath and then
runs revert, which removes local_abspath from revision control. Then
you release the lock on local_abspath which is no longer a versioned
directory. Inside svn_wc__release_write_lock the first check on kind
gets svn_wc__db_kind_unknown so the code then gets the parent and
releases locks from there. The recently reverted directory is no
longer a child of the parent, isn't locked and so doesn't get a lock
released. In effect the release call is releasing a completely
different set of locks to those acquired earlier in the function.
That's not the right thing to do is it?
--
Philip
