Continuing issue #749. Make locks associate together in groups instead of in parent-child hierarchies. Parent-child is still used when closing. Directory locking is now separate from committable harvesting. * subversion/include/svn_wc.h (svn_wc_adm_open, svn_wc_adm_retrieve, svn_wc_adm_close): Document new behavior. * subversion/libsvn_wc/wc.h (struct svn_wc_adm_access_t): Remove parent member. Rename children member to set. * subversion/libsvn_wc/lock.c (svn_wc__adm_access_child): Deleted. (svn_wc__adm_access_alloc): Remove parent parameter and change all callers. (svn_wc_adm_open): Add the associated access baton to the hash after creating the hash. Use temporary hash for opening children. (svn_wc_adm_retrieve): Don't call svn_wc__adm_access_child. (svn_wc__do_adm_close): Copy children to array before closing them. * subversion/libsvn_client/client.h (svn_client__harvest_committables, svn_client__get_copy_committables): Remove locked_dirs parameter. * subversion/libsvn_client/commit_util.c (lock_dir): Deleted. (harvest_committables): Revert 2554 for this file. Also remove locked_dirs parameter, and remove all directory locking. (svn_client__harvest_committables, svn_client__get_copy_committables): Remove locked_dirs parameter and directory locking. * subversion/libsvn_client/commit.c (unlock_dirs): Deleted. (svn_client_commit): Remove locked_dirs. Open an access baton. Close the baton intead of calling unlock_dirs. * subversion/libsvn_client/copy.c (unlock_dirs): Deleted. (wc_to_repos_copy): Remove locked_dirs. Open an access baton. Close the baton intead of calling unlock_dirs. git-svn-id: https://svn.apache.org/repos/asf/subversion/branches/issue-749-dev@842699 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/subversion/include/svn_wc.h b/subversion/include/svn_wc.h index b58df44..3dab4a9 100644 --- a/subversion/include/svn_wc.h +++ b/subversion/include/svn_wc.h
@@ -53,66 +53,67 @@ /*** Locking/Opening/Closing ***/ /* Baton for access to working copy administrative area. One day all such - access will require a baton, we're not there yet. */ + access will require a baton, we're not there yet. + + Access batons can be grouped into sets, by passing an existing open + baton when opening a new baton. Given one baton in a set, other batons + may be retrieved. This allows an entire hierarchy to be locked, and + then the set of batons can be passed around by passing a single baton. + */ typedef struct svn_wc_adm_access_t svn_wc_adm_access_t; /* Return an access baton in ADM_ACCESS for the working copy administrative area associated with the directory PATH. If WRITE_LOCK is set the baton will include a write lock, otherwise the baton can only be used for read - access. + access. If PATH refers to a directory that is already locked then the + error SVN_ERR_WC_LOCKED will be returned. - If PARENT_ACCESS is an open access baton PATH must refer to a - subdirectory of the PARENT_ACCESS directory, and then ADM_ACCESS will be - a child of PARENT_ACCESS. svn_wc_adm_close can be used to close - ADM_ACCESS. If ADM_ACCESS has not been closed when PARENT_ACCESS is - closed, it wil be closed automatically at that stage. - - If PARENT_ACCESS is an open access baton, and PATH refers to a - subdirectory that has already been opened as a child of PARENT_ACCESS - then the error SVN_ERR_WC_LOCKED will be returned. - - PARENT_ACCESS can be NULL, in which case ADM_ACCESS is not a child of - any other baton and must be explicitly closed. + If ASSOCIATED is an open access baton then ADM_ACCESS will be added to + the set containing ASSOCIATED. ASSOCIATED can be NULL, in which case + ADM_ACCESS is the start of a new set. If TREE_LOCK is TRUE then the working copy directory hierarchy under - PATH will be locked. At each level batons for the subdirectories will be - children of the parent directory's baton. This is an all-or-nothing - option, if it is not possible to lock the entire tree then an error will - be returned, there will be no locks and ADM_ACCESS will not be valid. + PATH will be locked. All the access batons will become part of the set + containing ADM_ACCESS. This is an all-or-nothing option, if it is not + possible to lock the entire tree then an error will be returned and + ADM_ACCESS will be invalid. POOL will be used to allocate memory for the baton and any subsequently cached items. If ADM_ACCESS has not been closed when the pool is - cleared, it will be closed automatically at the point but any physical - lock will remain in the working copy. */ + cleared, it will be closed automatically at that point, and removed from + its set. A baton closed in this way will not remove physical locks from + the working copy. */ svn_error_t *svn_wc_adm_open (svn_wc_adm_access_t **adm_access, - svn_wc_adm_access_t *parent_access, + svn_wc_adm_access_t *associated, const char *path, svn_boolean_t write_lock, svn_boolean_t tree_lock, apr_pool_t *pool); -/* Return an access baton in ADM_ACCESS associated with PATH. PATH must be - a subdirectory of the PARENT_ACCESS directory, and must have been - previously opened with PARENT_ACCESS as a parent. - POOL is used only for local processing it is not used for the batons. +/* Return an access baton in ADM_ACCESS associated with PATH. PATH must be + a directory that is locked and part of the set containing the ASSOCIATED + access baton. - ### I can't think of a good name for this function. */ + POOL is used only for local processing it is not used for the batons. */ svn_error_t *svn_wc_adm_retrieve (svn_wc_adm_access_t **adm_access, - svn_wc_adm_access_t *parent_access, + svn_wc_adm_access_t *associated, const char *path, apr_pool_t *pool); -/* Give up the access baton ADM_ACCESS, and its lock if any. It will also - close any child batons, for which this is a parent baton, that have not - yet been explicitly closed. Any physical locks will be removed from the - working copy. */ + +/* Give up the access baton ADM_ACCESS, and its lock if any. This will + recursively close any batons in the same set that are subdirectories of + ADM_ACCESS. Any physical locks will be removed from the working + copy. */ svn_error_t *svn_wc_adm_close (svn_wc_adm_access_t *adm_access); + /* Ensure ADM_ACCESS has a write lock, and that it is still valid. Returns SVN_ERR_WC_NOT_LOCKED if this is not the case. */ svn_error_t *svn_wc_adm_write_check (svn_wc_adm_access_t *adm_access); + /* Set *LOCKED to non-zero if PATH is locked, else set it to zero. */ svn_error_t *svn_wc_locked (svn_boolean_t *locked, const char *path,
diff --git a/subversion/libsvn_client/client.h b/subversion/libsvn_client/client.h index c7a1eb5..b93c5f9 100644 --- a/subversion/libsvn_client/client.h +++ b/subversion/libsvn_client/client.h
@@ -329,7 +329,6 @@ found in TARGETS will not be crawled for modifications. */ svn_error_t * svn_client__harvest_committables (apr_hash_t **committables, - apr_hash_t **locked_dirs, const char *parent_dir, apr_array_header_t *targets, svn_boolean_t nonrecursive, @@ -344,7 +343,6 @@ to a new repository URL (NEW_URL). */ svn_error_t * svn_client__get_copy_committables (apr_hash_t **committables, - apr_hash_t **locked_dirs, const char *new_url, const char *target, apr_pool_t *pool);
diff --git a/subversion/libsvn_client/commit.c b/subversion/libsvn_client/commit.c index 87a7bc8..5f35078 100644 --- a/subversion/libsvn_client/commit.c +++ b/subversion/libsvn_client/commit.c
@@ -605,32 +605,6 @@ static svn_error_t * -unlock_dirs (apr_hash_t *locked_dirs, - apr_pool_t *pool) -{ - apr_hash_index_t *hi; - - /* Split if there's nothing to be done. */ - if (! locked_dirs) - return SVN_NO_ERROR; - - /* Clean up any locks. */ - for (hi = apr_hash_first (pool, locked_dirs); hi; hi = apr_hash_next (hi)) - { - const void *key; - void *val; - svn_wc_adm_access_t *adm_access; - - apr_hash_this (hi, &key, NULL, &val); - adm_access = val; - SVN_ERR (svn_wc_adm_close (adm_access)); - } - - return SVN_NO_ERROR; -} - - -static svn_error_t * remove_tmpfiles (apr_hash_t *tempfiles, apr_pool_t *pool) { @@ -747,7 +721,8 @@ const char *base_dir; const char *base_url; apr_array_header_t *rel_targets; - apr_hash_t *committables, *locked_dirs, *tempfiles = NULL; + apr_hash_t *committables, *tempfiles = NULL; + svn_wc_adm_access_t *base_dir_access; apr_array_header_t *commit_items; apr_status_t apr_err = 0; apr_file_t *xml_hnd = NULL; @@ -755,6 +730,7 @@ svn_error_t *bump_err = NULL, *cleanup_err = NULL; svn_boolean_t use_xml = (xml_dst && xml_dst[0]) ? TRUE : FALSE; svn_boolean_t commit_in_progress = FALSE; + svn_wc_entry_t *base_entry; const char *display_dir = "."; int notify_path_offset; int i; @@ -762,6 +738,21 @@ /* Condense the target list. */ SVN_ERR (svn_path_condense_targets (&base_dir, &rel_targets, targets, pool)); + /* Eeek! To get an access baton we need a directory name. If the user + explicitly commits a deleted file that has been physically removed + from the working copy (as is normal for deleted files), then base_dir + will be the file name not a directory name. To identify this case we + need to get the entry for base_dir before we get the access baton. I + wonder where this logic should live? */ + SVN_ERR (svn_wc_entry (&base_entry, base_dir, TRUE, pool)); + if (base_entry->kind == svn_node_dir) + SVN_ERR (svn_wc_adm_open (&base_dir_access, NULL, base_dir, TRUE, TRUE, + pool)); + else + SVN_ERR (svn_wc_adm_open (&base_dir_access, NULL, + svn_path_remove_component_nts (base_dir, pool), + TRUE, TRUE, pool)); + /* If we calculated only a base_dir and no relative targets, this must mean that we are being asked to commit a single directory. In order to do this properly, we need to anchor our commit up one @@ -790,7 +781,6 @@ /* Crawl the working copy for commit items. */ if ((cmt_err = svn_client__harvest_committables (&committables, - &locked_dirs, base_dir, rel_targets, nonrecursive, @@ -917,13 +907,9 @@ adm_access_path = item->path; else svn_path_split_nts (item->path, &adm_access_path, NULL, pool); - adm_access = apr_hash_get (locked_dirs, adm_access_path, - APR_HASH_KEY_STRING); - if (! adm_access) - /* This should *never* happen */ - return svn_error_createf(SVN_ERR_GENERAL, 0, NULL, pool, - "BUG: no lock for %s", - item->path); + if ((bump_err = svn_wc_adm_retrieve (&adm_access, base_dir_access, + adm_access_path, pool))) + goto cleanup; if ((item->state_flags & SVN_CLIENT_COMMIT_ITEM_ADD) && (item->kind == svn_node_dir) @@ -948,10 +934,6 @@ svn_pool_destroy (subpool); } - /* Unlock the locked directories. */ - if (! ((unlock_err = unlock_dirs (locked_dirs, pool)))) - locked_dirs = NULL; - /* If we were committing into XML, close the xml file. */ if (use_xml) { @@ -980,9 +962,8 @@ if (commit_in_progress) editor->abort_edit (edit_baton); /* ignore return value */ - /* Unlock any remaining locked dirs. */ - if (locked_dirs) - unlock_err = unlock_dirs (locked_dirs, pool); + /* ### Under what conditions should we remove the locks? */ + unlock_err = svn_wc_adm_close (base_dir_access); /* Remove any outstanding temporary text-base files. */ cleanup_err = remove_tmpfiles (tempfiles, pool);
diff --git a/subversion/libsvn_client/commit_util.c b/subversion/libsvn_client/commit_util.c index 05fbb73..f9c6a7d 100644 --- a/subversion/libsvn_client/commit_util.c +++ b/subversion/libsvn_client/commit_util.c
@@ -47,35 +47,6 @@ /*** Harvesting Commit Candidates ***/ -/* If DIR isn't already in the LOCKED_DIRS hash, attempt to lock it. - If the lock is successful, add DIR to the LOCKED_DIRS hash. Use - the hash's pool for adding new items, and POOL for any other - allocations. */ -static svn_error_t * -lock_dir (apr_hash_t *locked_dirs, - const char *dir, - apr_pool_t *pool) -{ - apr_pool_t *hash_pool = apr_hash_pool_get (locked_dirs); - - if (! apr_hash_get (locked_dirs, dir, APR_HASH_KEY_STRING)) - { - /* Short lived, don't use hash pool */ - const char *parent_dir = svn_path_remove_component_nts (dir, pool); - svn_wc_adm_access_t *adm_access, *parent_access; - - /* Get parent, NULL is OK */ - parent_access = apr_hash_get (locked_dirs, parent_dir, - APR_HASH_KEY_STRING); - SVN_ERR (svn_wc_adm_open (&adm_access, parent_access, dir, TRUE, FALSE, - hash_pool)); - apr_hash_set (locked_dirs, apr_pstrdup (hash_pool, dir), - APR_HASH_KEY_STRING, adm_access); - } - return SVN_NO_ERROR; -} - - /* Add a new commit candidate (described by all parameters except `COMMITTABLES') to the COMMITABLES hash. */ static void @@ -137,7 +108,6 @@ added with history as URL. */ static svn_error_t * harvest_committables (apr_hash_t *committables, - apr_hash_t *locked_dirs, const char *path, const char *url, const char *copyfrom_url, @@ -146,7 +116,6 @@ svn_boolean_t adds_only, svn_boolean_t copy_mode, svn_boolean_t nonrecursive, - svn_boolean_t lock_subdirs, apr_pool_t *pool) { apr_pool_t *subpool = svn_pool_create (pool); /* ### why? */ @@ -299,37 +268,25 @@ /* Now, if this is something to commit, add it to our list. */ if (state_flags) { - /* If the commit item is a directory, lock it, else lock its parent. */ - if (entry->kind == svn_node_dir) - /* ### Need to lock parent here as well? We will need to modify the - ### parents entry file if this is not the root */ - SVN_ERR (lock_dir (locked_dirs, path, pool)); - else - SVN_ERR (lock_dir (locked_dirs, p_path, pool)); - /* Finally, add the committable item. */ add_committable (committables, path, entry->kind, url, cf_url ? entry->copyfrom_rev : entry->revision, cf_url, state_flags); - - /* We need to lock all directories to enable post-commit - processing of the working copy */ - lock_subdirs = TRUE; } - if (entries) + /* For directories, recursively handle each of their entries (except + when the directory is being deleted, unless the deletion is part + of a replacement ... how confusing). */ + if ((entries) + && ((! (state_flags & SVN_CLIENT_COMMIT_ITEM_DELETE)) + || (state_flags & SVN_CLIENT_COMMIT_ITEM_ADD))) { apr_hash_index_t *hi; svn_wc_entry_t *this_entry; apr_pool_t *loop_pool = svn_pool_create (subpool); /* Loop over all other entries in this directory, skipping the - "this dir" entry. - - ### May need to do this twice, if we need to do sub-directories, - ### before files. Subdirectories may require us to lock the - ### parent, files only lock this dir, and doing them in the wrong - ### order may break the parent-child lock relationship. */ + "this dir" entry. */ for (hi = apr_hash_first (subpool, entries); hi; hi = apr_hash_next (hi)) @@ -372,16 +329,6 @@ if (this_cf_url) this_cf_url = svn_path_join (this_cf_url, name, loop_pool); - if (lock_subdirs && this_entry->kind == svn_node_dir) - SVN_ERR (lock_dir (locked_dirs, full_path, pool)); - - /* For directories, recursively handle each of their entries - (except when the directory is being deleted, unless the - deletion is part of a replacement ... how confusing). */ - if ((state_flags & SVN_CLIENT_COMMIT_ITEM_DELETE) - && ! (state_flags & SVN_CLIENT_COMMIT_ITEM_ADD)) - continue; - /* We'll use the entry's URL if it has one and if we aren't in copy_mode, else, we'll just extend the parent's URL with the entry's basename. */ @@ -393,7 +340,7 @@ /* Recurse. */ SVN_ERR (harvest_committables - (committables, locked_dirs, full_path, + (committables, full_path, used_url ? used_url : this_entry->url, this_cf_url, (svn_wc_entry_t *)val, @@ -401,7 +348,6 @@ adds_only, copy_mode, FALSE, - lock_subdirs, subpool)); svn_pool_clear (loop_pool); @@ -416,7 +362,6 @@ svn_error_t * svn_client__harvest_committables (apr_hash_t **committables, - apr_hash_t **locked_dirs, const char *parent_dir, apr_array_header_t *targets, svn_boolean_t nonrecursive, @@ -427,9 +372,6 @@ /* Create the COMMITTABLES hash. */ *committables = apr_hash_make (pool); - /* Create the LOCKED_DIRS hash. */ - *locked_dirs = apr_hash_make (pool); - /* ### Would be nice to use an iteration pool here, but need to first look into lifetime issues w/ anything passed to harvest_committables() and possibly stored by it. */ @@ -516,9 +458,9 @@ target); /* Handle our TARGET. */ - SVN_ERR (harvest_committables (*committables, *locked_dirs, target, + SVN_ERR (harvest_committables (*committables, target, url, NULL, entry, NULL, FALSE, FALSE, - nonrecursive, FALSE, pool)); + nonrecursive, pool)); i++; } @@ -530,7 +472,6 @@ svn_error_t * svn_client__get_copy_committables (apr_hash_t **committables, - apr_hash_t **locked_dirs, const char *new_url, const char *target, apr_pool_t *pool) @@ -540,9 +481,6 @@ /* Create the COMMITTABLES hash. */ *committables = apr_hash_make (pool); - /* Create the LOCKED_DIRS hash. */ - *locked_dirs = apr_hash_make (pool); - /* Read the entry for TARGET. */ SVN_ERR (svn_wc_entry (&entry, target, FALSE, pool)); if (! entry) @@ -550,9 +488,9 @@ (SVN_ERR_ENTRY_NOT_FOUND, 0, NULL, pool, target); /* Handle our TARGET. */ - SVN_ERR (harvest_committables (*committables, *locked_dirs, target, + SVN_ERR (harvest_committables (*committables, target, new_url, entry->url, entry, NULL, - FALSE, TRUE, FALSE, FALSE, pool)); + FALSE, TRUE, FALSE, pool)); return SVN_NO_ERROR; }
diff --git a/subversion/libsvn_client/copy.c b/subversion/libsvn_client/copy.c index 437a0fb..dc4eaeb 100644 --- a/subversion/libsvn_client/copy.c +++ b/subversion/libsvn_client/copy.c
@@ -362,32 +362,6 @@ static svn_error_t * -unlock_dirs (apr_hash_t *locked_dirs, - apr_pool_t *pool) -{ - apr_hash_index_t *hi; - - /* Split if there's nothing to be done. */ - if (! locked_dirs) - return SVN_NO_ERROR; - - /* Clean up any locks. */ - for (hi = apr_hash_first (pool, locked_dirs); hi; hi = apr_hash_next (hi)) - { - const void *key; - void *val; - svn_wc_adm_access_t *adm_access; - - apr_hash_this (hi, &key, NULL, &val); - adm_access = val; - SVN_ERR (svn_wc_adm_close (adm_access)); - } - - return SVN_NO_ERROR; -} - - -static svn_error_t * remove_tmpfiles (apr_hash_t *tempfiles, apr_pool_t *pool) { @@ -489,7 +463,8 @@ svn_revnum_t committed_rev = SVN_INVALID_REVNUM; const char *committed_date = NULL; const char *committed_author = NULL; - apr_hash_t *committables, *locked_dirs, *tempfiles = NULL; + apr_hash_t *committables, *tempfiles = NULL; + svn_wc_adm_access_t *adm_access; apr_array_header_t *commit_items; svn_error_t *cmt_err = NULL, *unlock_err = NULL, *cleanup_err = NULL; svn_boolean_t commit_in_progress = FALSE; @@ -504,6 +479,9 @@ if (svn_path_is_empty_nts (parent)) parent = "."; + /* ### Do we need locks for a wc->repos copy? */ + SVN_ERR (svn_wc_adm_open (&adm_access, NULL, parent, TRUE, TRUE, pool)); + /* Split the DST_URL into an anchor and target. */ svn_path_split_nts (dst_url, &anchor, &target, pool); @@ -551,7 +529,6 @@ /* Crawl the working copy for commit items. */ if ((cmt_err = svn_client__get_copy_committables (&committables, - &locked_dirs, base_url, base_path, pool))) @@ -608,9 +585,8 @@ if (session) ra_lib->close (session); - /* Unlock any remaining locked dirs. */ - if (locked_dirs) - unlock_err = unlock_dirs (locked_dirs, pool); + /* ### Under what conditions should we remove the locks? */ + unlock_err = svn_wc_adm_close (adm_access); /* Remove any outstanding temporary text-base files. */ if (tempfiles)
diff --git a/subversion/libsvn_wc/lock.c b/subversion/libsvn_wc/lock.c index 668e389..960d479 100644 --- a/subversion/libsvn_wc/lock.c +++ b/subversion/libsvn_wc/lock.c
@@ -93,14 +93,12 @@ static svn_wc_adm_access_t * svn_wc__adm_access_alloc (enum svn_wc__adm_access_type type, - svn_wc_adm_access_t *parent, const char *path, apr_pool_t *pool) { svn_wc_adm_access_t *lock = apr_palloc (pool, sizeof (*lock)); lock->type = type; - lock->parent = parent; - lock->children = NULL; + lock->set = NULL; lock->lock_exists = FALSE; /* ### Some places lock with a path that is not canonical, we need ### cannonical paths for reliable parent-child determination */ @@ -120,8 +118,7 @@ { svn_error_t *err; svn_wc_adm_access_t *lock - = svn_wc__adm_access_alloc (svn_wc__adm_access_write_lock, NULL, path, - pool); + = svn_wc__adm_access_alloc (svn_wc__adm_access_write_lock, path, pool); err = svn_wc__lock (lock, 0, pool); if (err) @@ -137,33 +134,9 @@ return SVN_NO_ERROR; } -static svn_error_t * -svn_wc__adm_access_child (svn_wc_adm_access_t **adm_access, - svn_wc_adm_access_t *parent_access, - const char *path, - apr_pool_t *pool) -{ - - /* PATH must be a child. We only need the result, not the allocated - name. */ - const char *name = svn_path_is_child (parent_access->path, path, pool); - if (!name) - return svn_error_createf (SVN_ERR_WC_INVALID_LOCK, 0, NULL, pool, - "lock path is not a child (%s)", - path); - - if (parent_access->children) - *adm_access = apr_hash_get (parent_access->children, path, - APR_HASH_KEY_STRING); - else - *adm_access = NULL; - - return SVN_NO_ERROR; -} - svn_error_t * svn_wc_adm_open (svn_wc_adm_access_t **adm_access, - svn_wc_adm_access_t *parent_access, + svn_wc_adm_access_t *associated, const char *path, svn_boolean_t write_lock, svn_boolean_t tree_lock, @@ -171,12 +144,14 @@ { svn_wc_adm_access_t *lock; - if (parent_access) + if (associated && associated->set) { - SVN_ERR (svn_wc__adm_access_child (&lock, parent_access, path, - pool)); + lock = apr_hash_get (associated->set, path, APR_HASH_KEY_STRING); if (lock) - /* Already locked */ + /* Already locked. The reason we don't return the existing baton + here is that the user is supposed to know whether a directory is + locked: if it's not locked call svn_wc_adm_open, if it is locked + call svn_wc_adm_retrieve. */ return svn_error_createf (SVN_ERR_WC_LOCKED, 0, NULL, pool, "directory already locked (%s)", path); @@ -185,8 +160,8 @@ /* Need to create a new lock */ if (write_lock) { - lock = svn_wc__adm_access_alloc (svn_wc__adm_access_write_lock, - parent_access, path, pool); + lock = svn_wc__adm_access_alloc (svn_wc__adm_access_write_lock, path, + pool); SVN_ERR (svn_wc__lock (lock, 0, pool)); lock->lock_exists = TRUE; } @@ -201,36 +176,36 @@ "lock path is not a directory (%s)", path); - lock = svn_wc__adm_access_alloc (svn_wc__adm_access_unlocked, - parent_access, path, pool); + lock = svn_wc__adm_access_alloc (svn_wc__adm_access_unlocked, path, pool); } - lock->parent = parent_access; - if (lock->parent) + if (associated) { - if (! lock->parent->children) - lock->parent->children = apr_hash_make (lock->parent->pool); + if (! associated->set) + { + associated->set = apr_hash_make (associated->pool); + apr_hash_set (associated->set, associated->path, APR_HASH_KEY_STRING, + associated); + } -#if 0 - /* ### Need to think about this. Creating an empty APR hash allocates - ### several hundred bytes. Since the hash key is the complete - ### path, children for different parents cannot clash. Can we save - ### memory by reusing the parent's hash? */ - lock->children = lock->parent->children; -#endif - - apr_hash_set (lock->parent->children, lock->path, APR_HASH_KEY_STRING, - lock); + lock->set = associated->set; + apr_hash_set (lock->set, lock->path, APR_HASH_KEY_STRING, lock); } if (tree_lock) { - /* ### Could use this code to initialise the cache, in which case the - ### subpool should be removed. */ + /* ### Use this code to initialise the cache? */ apr_hash_t *entries; apr_hash_index_t *hi; apr_pool_t *subpool = svn_pool_create (pool); + SVN_ERR (svn_wc_entries_read (&entries, path, FALSE, subpool)); + + /* Use a temporary hash until all children have been opened. */ + if (associated) + lock->set = apr_hash_make (subpool); + + /* Open the tree */ for (hi = apr_hash_first (subpool, entries); hi; hi = apr_hash_next (hi)) { void *val; @@ -246,16 +221,39 @@ continue; entry_path = svn_path_join (lock->path, entry->name, subpool); - /* Use the main lock's pool here, it needs to persist */ + /* Don't use the subpool pool here, the lock needs to persist */ svn_err = svn_wc_adm_open (&entry_access, lock, entry_path, write_lock, tree_lock, lock->pool); if (svn_err) { - /* Give up any locks we have acquired. */ + /* This closes all the children in temporary hash as well */ svn_wc_adm_close (lock); + svn_pool_destroy (subpool); return svn_err; } } + + /* Switch from temporary hash to permanent hash */ + if (associated) + { + for (hi = apr_hash_first (subpool, lock->set); + hi; + hi = apr_hash_next (hi)) + { + const void *key; + void *val; + const char *entry_path; + svn_wc_adm_access_t *entry_access; + + apr_hash_this (hi, &key, NULL, &val); + entry_path = key; + entry_access = val; + apr_hash_set (associated->set, entry_path, APR_HASH_KEY_STRING, + entry_access); + entry_access->set = associated->set; + } + lock->set = associated->set; + } svn_pool_destroy (subpool); } @@ -265,11 +263,16 @@ svn_error_t * svn_wc_adm_retrieve (svn_wc_adm_access_t **adm_access, - svn_wc_adm_access_t *parent_access, + svn_wc_adm_access_t *associated, const char *path, apr_pool_t *pool) { - SVN_ERR (svn_wc__adm_access_child (adm_access, parent_access, path, pool)); + if (associated->set) + *adm_access = apr_hash_get (associated->set, path, APR_HASH_KEY_STRING); + else if (! strcmp (associated->path, path)) + *adm_access = associated; + else + *adm_access = NULL; if (! *adm_access) return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, 0, NULL, pool, "directory not locked (%s)", @@ -287,17 +290,39 @@ svn_wc__adm_access_pool_cleanup); /* Close children */ - if (adm_access->children) + if (adm_access->set) { - for (hi = apr_hash_first (adm_access->pool, adm_access->children); + /* The documentation says that modifying a hash while iterating over + it is allowed but unpredictable! So, first loop to identify and + copy children, second loop to close them. */ + int i; + apr_array_header_t *children = apr_array_make (adm_access->pool, 1, + sizeof (adm_access)); + + for (hi = apr_hash_first (adm_access->pool, adm_access->set); hi; hi = apr_hash_next (hi)) { void *val; - svn_wc_adm_access_t *child_access; + svn_wc_adm_access_t *associated; + const char *name; apr_hash_this (hi, NULL, NULL, &val); - child_access = val; - SVN_ERR (svn_wc__do_adm_close (child_access, preserve_lock)); + associated = val; + name = svn_path_is_child (adm_access->path, associated->path, + adm_access->pool); + if (name) + { + *(svn_wc_adm_access_t**)apr_array_push (children) = associated; + /* Deleting current element is allowed and predictable */ + apr_hash_set (adm_access->set, associated->path, + APR_HASH_KEY_STRING, NULL); + } + } + for (i = 0; i < children->nelts; ++i) + { + svn_wc_adm_access_t *child = APR_ARRAY_IDX(children, i, + svn_wc_adm_access_t*); + SVN_ERR (svn_wc__do_adm_close (child, preserve_lock)); } } @@ -313,10 +338,9 @@ adm_access->type = svn_wc__adm_access_closed; } - /* Detach from parent */ - if (adm_access->parent) - apr_hash_set (adm_access->parent->children, adm_access->path, - APR_HASH_KEY_STRING, NULL); + /* Detach from set */ + if (adm_access->set) + apr_hash_set (adm_access->set, adm_access->path, APR_HASH_KEY_STRING, NULL); return SVN_NO_ERROR; }
diff --git a/subversion/libsvn_wc/wc.h b/subversion/libsvn_wc/wc.h index c087a74..9f7300f 100644 --- a/subversion/libsvn_wc/wc.h +++ b/subversion/libsvn_wc/wc.h
@@ -188,12 +188,9 @@ apr_hash_t *entries; #endif - /* PARENT access baton, may be NULL. */ - svn_wc_adm_access_t *parent; - - /* CHILDREN is a hash of svn_wc_adm_access_t* keyed on char* - representing the path to sub-directories that are also locked. */ - apr_hash_t *children; + /* SET is a hash of svn_wc_adm_access_t* keyed on char* representing the + path to other directories that are also locked. */ + apr_hash_t *set; /* POOL is used to allocate cached items, they need to persist for the lifetime of this access baton */