Re: svn commit: r36320 - in trunk/subversion: libsvn_subr libsvn_wc

["Hyrum K. Wright" <hy...@mail.utexas.edu>]

Greg, et al,
FYI, after this commit, I see:

Summary of test results:
   1011 tests PASSED
   24 tests SKIPPED
   24 tests XFAILED
   88 tests FAILED
   2 tests XPASSED

which is slightly worse than it was previously.  Many of those  
failures, though, are due to the assertion in svn_wc__close() which  
checks to make sure the sdbs for pdh's with the same wcroot_abspath  
are the same.  Without this assertion, I get:

Summary of test results:
   1024 tests PASSED
   24 tests SKIPPED
   24 tests XFAILED
   75 tests FAILED
   2 tests XPASSED

which is better, but the assertions are still indicative of a bug  
somewhere.

-Hyrum

On Mar 4, 2009, at 1:24 PM, Hyrum K. Wright wrote:

> Author: hwright
> Date: Wed Mar  4 11:24:13 2009
> New Revision: 36320
>
> Log:
> Close the wc_db when closing it's associated access baton.  Also  
> close the
> underlying sqlite dbs when closing the wc_db.  This fixes several  
> bugs where
> svn complains about having too many open files.
>
> * subversion/libsvn_subr/sqlite.c
>  (close_apr): Just no-op if the db has already been closed.
>
> * subversion/libsvn_wc/lock.c
>  (do_close): If the associated set for the baton is empty, close the
>    shared wc_db object.
>
> * subversion/libsvn_wc/wc_db.c
>  (svn_wc__db_close): Implement.
>
> Modified:
>   trunk/subversion/libsvn_subr/sqlite.c
>   trunk/subversion/libsvn_wc/lock.c
>   trunk/subversion/libsvn_wc/wc_db.c
>
> Modified: trunk/subversion/libsvn_subr/sqlite.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/sqlite.c?pathrev=36320&r1=36319&r2=36320
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- trunk/subversion/libsvn_subr/sqlite.c	Wed Mar  4 11:18:57 2009	 
> (r36319)
> +++ trunk/subversion/libsvn_subr/sqlite.c	Wed Mar  4 11:24:13 2009	 
> (r36320)
> @@ -729,7 +729,9 @@ close_apr(void *data)
>   int result;
>   int i;
>
> -  SVN_ERR_ASSERT_NO_RETURN(db->db3 != NULL);
> +  /* Check to see if we've already closed this database. */
> +  if (db->db3 == NULL)
> +    return APR_SUCCESS;
>
>   /* Finalize any existing prepared statements. */
>   for (i = 0; i < db->nbr_statements; i++)
>
> Modified: trunk/subversion/libsvn_wc/lock.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/lock.c?pathrev=36320&r1=36319&r2=36320
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- trunk/subversion/libsvn_wc/lock.c	Wed Mar  4 11:18:57 2009	 
> (r36319)
> +++ trunk/subversion/libsvn_wc/lock.c	Wed Mar  4 11:24:13 2009	 
> (r36320)
> @@ -1297,6 +1297,14 @@ do_close(svn_wc_adm_access_t *adm_access
>                      || apr_hash_count(adm_access->shared->set) == 0);
>     }
>
> +  /* Close the underlying wc_db. */
> +  if (adm_access->set_owner)
> +    {
> +      SVN_ERR_ASSERT(apr_hash_count(adm_access->shared->set) == 0);
> +      if (adm_access->shared->db)
> +        SVN_ERR(svn_wc__db_close(adm_access->shared->db,  
> scratch_pool));
> +    }
> +
>   return SVN_NO_ERROR;
> }
>
>
> Modified: trunk/subversion/libsvn_wc/wc_db.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/wc_db.c?pathrev=36320&r1=36319&r2=36320
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- trunk/subversion/libsvn_wc/wc_db.c	Wed Mar  4 11:18:57 2009	 
> (r36319)
> +++ trunk/subversion/libsvn_wc/wc_db.c	Wed Mar  4 11:24:13 2009	 
> (r36320)
> @@ -1194,7 +1194,42 @@ svn_error_t *
> svn_wc__db_close(svn_wc__db_t *db,
>                  apr_pool_t *scratch_pool)
> {
> -  NOT_IMPLEMENTED();
> +  apr_hash_t *sdbs = apr_hash_make(scratch_pool);
> +  apr_hash_index_t *hi;
> +
> +  /* We may have sdbs shared between pdhs, so put them all in a  
> hash to
> +     collapse them, validating along the way. */
> +  for (hi = apr_hash_first(scratch_pool, db->dir_data); hi;
> +        hi = apr_hash_next(hi))
> +    {
> +      void *val;
> +      svn_wc__db_pdh_t *pdh;
> +      svn_wc__db_pdh_t *existing_pdh;
> +
> +      apr_hash_this(hi, NULL, NULL, &val);
> +      pdh = val;
> +
> +      existing_pdh = apr_hash_get(sdbs, pdh->wcroot_abspath,
> +                                  APR_HASH_KEY_STRING);
> +      if (existing_pdh)
> +        SVN_ERR_ASSERT(existing_pdh->sdb == pdh->sdb);
> +
> +      apr_hash_set(sdbs, pdh->wcroot_abspath, APR_HASH_KEY_STRING,  
> pdh->sdb);
> +    }
> +
> +  /* Now close all of the non-duplicate databases. */
> +  for (hi = apr_hash_first(scratch_pool, sdbs); hi; hi =  
> apr_hash_next(hi))
> +    {
> +      void *val;
> +      svn_sqlite__db_t *sdb;
> +
> +      apr_hash_this(hi, NULL, NULL, &val);
> +      sdb = val;
> +
> +      SVN_ERR(svn_sqlite__close(sdb));
> +    }
> +
> +  return SVN_NO_ERROR;
> }
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=1268095

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1268445