John Szakmeister wrote:
> On Tuesday 02 December 2003 17:45, Julian Foad wrote:
>
>>I am also playing with the following patch:
>>
>>Index: subversion/libsvn_wc/lock.c
>>===================================================================
>>--- subversion/libsvn_wc/lock.c (revision 7899)
>>+++ subversion/libsvn_wc/lock.c (working copy)
>>@@ -738,6 +738,9 @@
>> assert (! adm_access->set_owner || apr_hash_count (adm_access->set) == 0); }
>>
>>+ /* Invalidate the baton so that attempts to use it will be caught. */
>>+ memset (adm_access, 0, sizeof(*adm_access));
>>+
>> return SVN_NO_ERROR;
>> }
>>
>>
>>It catches the error that is the subject of this message, but makes lots of
>>other tests fail too. I'm still investigating whether it is finding real
>>bugs or is a wrong thing to do.
>
> Interesting that this caused other errors. I'm curious to know what you
> find! :-)
It does catch a few other instances of using an adm_access baton after it was closed. The attached patch fixes all but one tricky case, marked with "###". Do you fancy trying to see how to deal with that tricky case?
- Julian
Avoid using an adm_access baton after it has been closed.
* subversion/libsvn_wc/log.c (svn_wc__run_log):
* subversion/libsvn_wc/adm_ops.c (svn_wc_process_committed ###,
svn_wc_remove_from_revision_control):
Use adm_access before (rather than after) doing something that closes it.
* subversion/libsvn_wc/lock.c
(do_close): Invalidate the baton so that mis-use may be caught.
(svn_wc_locked, svn_wc_adm_access_path):
Index: subversion/libsvn_wc/log.c
===================================================================
--- subversion/libsvn_wc/log.c (revision 7917)
+++ subversion/libsvn_wc/log.c (working copy)
@@ -1292,6 +1292,16 @@ svn_wc__run_log (svn_wc_adm_access_t *ad
{
const svn_wc_entry_t *thisdir_entry, *parent_entry;
svn_wc_entry_t tmp_entry;
+ const char *parent, *bname;
+ svn_wc_adm_access_t *parent_access;
+
+ svn_path_split (svn_wc_adm_access_path (adm_access), &parent,
+ &bname, pool);
+ SVN_ERR (svn_wc_adm_retrieve (&parent_access, adm_access, parent,
+ pool));
+ SVN_ERR (svn_wc_entry (&parent_entry, parent, parent_access, FALSE,
+ pool));
+
SVN_ERR (svn_wc_entry (&thisdir_entry,
svn_wc_adm_access_path (adm_access), adm_access,
FALSE, pool));
@@ -1307,29 +1317,17 @@ svn_wc__run_log (svn_wc_adm_access_t *ad
/* If revnum of this dir is greater than parent's revnum, then
recreate 'deleted' entry in parent. */
- {
- const char *parent, *bname;
- svn_wc_adm_access_t *parent_access;
-
- svn_path_split (svn_wc_adm_access_path (adm_access), &parent,
- &bname, pool);
- SVN_ERR (svn_wc_adm_retrieve (&parent_access, adm_access, parent,
- pool));
- SVN_ERR (svn_wc_entry (&parent_entry, parent, parent_access, FALSE,
- pool));
-
- if (thisdir_entry->revision > parent_entry->revision)
- {
- tmp_entry.kind = svn_node_dir;
- tmp_entry.deleted = TRUE;
- tmp_entry.revision = thisdir_entry->revision;
- SVN_ERR (svn_wc__entry_modify (parent_access, bname, &tmp_entry,
- SVN_WC__ENTRY_MODIFY_REVISION
- | SVN_WC__ENTRY_MODIFY_KIND
- | SVN_WC__ENTRY_MODIFY_DELETED,
- TRUE, pool));
- }
- }
+ if (thisdir_entry->revision > parent_entry->revision)
+ {
+ tmp_entry.kind = svn_node_dir;
+ tmp_entry.deleted = TRUE;
+ tmp_entry.revision = thisdir_entry->revision;
+ SVN_ERR (svn_wc__entry_modify (parent_access, bname, &tmp_entry,
+ SVN_WC__ENTRY_MODIFY_REVISION
+ | SVN_WC__ENTRY_MODIFY_KIND
+ | SVN_WC__ENTRY_MODIFY_DELETED,
+ TRUE, pool));
+ }
}
else
{
Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/adm_ops.c (revision 7917)
+++ subversion/libsvn_wc/adm_ops.c (working copy)
@@ -399,7 +399,8 @@ svn_wc_process_committed (const char *pa
/* Run the log file we just created. */
SVN_ERR (svn_wc__run_log (adm_access, NULL, pool));
-
+ /* ### this may have removed the directory and so invalidated adm_access */
+
if (recurse)
{
apr_hash_t *entries;
@@ -1727,6 +1728,7 @@ svn_wc_remove_from_revision_control (svn
apr_pool_t *subpool = svn_pool_create (pool);
apr_hash_index_t *hi;
svn_wc_entry_t incomplete_entry;
+ const char *adm_access_path = svn_wc_adm_access_path (adm_access);
/* ### sanity check: check 2 places for DELETED flag? */
@@ -1867,8 +1869,7 @@ svn_wc_remove_from_revision_control (svn
*non*-recursive dir_remove should work. If it doesn't,
no big deal. Just assume there are unversioned items in
there and set "left_something" */
- err = svn_io_dir_remove_nonrecursive
- (svn_wc_adm_access_path (adm_access), subpool);
+ err = svn_io_dir_remove_nonrecursive (adm_access_path, subpool);
if (err)
{
left_something = TRUE;
Index: subversion/libsvn_wc/lock.c
===================================================================
--- subversion/libsvn_wc/lock.c (revision 7917)
+++ subversion/libsvn_wc/lock.c (working copy)
@@ -738,6 +738,9 @@ do_close (svn_wc_adm_access_t *adm_acces
assert (! adm_access->set_owner || apr_hash_count (adm_access->set) == 0);
}
+ /* Invalidate the baton so that attempts to use it will be caught. */
+ memset (adm_access, 0, sizeof(*adm_access));
+
return SVN_NO_ERROR;
}
@@ -810,6 +813,7 @@ svn_wc_locked (svn_boolean_t *locked, co
const char *
svn_wc_adm_access_path (svn_wc_adm_access_t *adm_access)
{
+ assert (adm_access->path);
return adm_access->path;
}
@@ -817,6 +821,7 @@ svn_wc_adm_access_path (svn_wc_adm_acces
apr_pool_t *
svn_wc_adm_access_pool (svn_wc_adm_access_t *adm_access)
{
+ assert (adm_access->pool);
return adm_access->pool;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Dec 3 04:17:03 2003