[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: svn commit: r35729 - trunk/subversion/libsvn_subr

From: Bert Huijben <rhuijben_at_sharpsvn.net>
Date: Sat, 7 Feb 2009 13:55:28 +0100

> -----Original Message-----
> From: Hyrum K. Wright [mailto:hyrum_at_hyrumwright.org]
> Sent: Saturday, February 07, 2009 12:32 AM
> To: svn_at_subversion.tigris.org
> Subject: svn commit: r35729 - trunk/subversion/libsvn_subr
>
> Author: hwright
> Date: Fri Feb 6 15:32:01 2009
> New Revision: 35729
>
> Log:
> Instead of guarding against multiple invocations of init_sqlite()
> externally,
> move the static variable and the checking of it's value internal. This
> also
> allows svn_sqlite__get_schema_version() to initialize the library.
>
> * subversion/libsvn_subr/sqlite.c
> (init_sqlite): Return immediately if the library has been
> initialized.
> (svn_sqlite__get_schema_version): Initialize the sqlite library.
> (svn_sqlite__open): Unconditionally initialize the sqlite library,
> since it
> now does internal invocation checking.
>
> Modified:
> trunk/subversion/libsvn_subr/sqlite.c
>
> Modified: trunk/subversion/libsvn_subr/sqlite.c
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/sqlite.c?
> pathrev=35729&r1=35728&r2=35729
> =======================================================================
> =======
> --- trunk/subversion/libsvn_subr/sqlite.c Fri Feb 6 14:48:51 2009
> (r35728)
> +++ trunk/subversion/libsvn_subr/sqlite.c Fri Feb 6 15:32:01 2009
> (r35729)
> @@ -461,6 +461,13 @@ check_format(svn_sqlite__db_t *db, int l
> static svn_error_t *
> init_sqlite(void)
> {
> + static svn_boolean_t sqlite_initialized = FALSE;
> +
> + /* There is a potential initialization race condition here, but
> + it currently isn't worth guarding against (e.g. with a mutex). */
> + if (sqlite_initialized)
> + return SVN_NO_ERROR;
> +

Please use the atomic initialization functions here... Users of
multithreaded applications will thank you :)

A second thread might just assume everything is OK before the first thread
actually completes the initialization.

        Bert

> if (sqlite3_libversion_number() < SQLITE_VERSION_NUMBER) {
> return svn_error_createf(SVN_ERR_SQLITE_ERROR, NULL,
> _("SQLite compiled for %s, but running
> with %s"),
> @@ -485,6 +492,7 @@ init_sqlite(void)
>
> /* NOTE: if more work is performed here, then consider using
> svn_atomic__init_once() for the call to this function. */
> + sqlite_initialized = TRUE;
>
> return SVN_NO_ERROR;
> }
> @@ -496,6 +504,7 @@ svn_sqlite__get_schema_version(int *vers
> {
> svn_sqlite__db_t db;
>
> + SVN_ERR(init_sqlite());
> SQLITE_ERR(sqlite3_open(path, &db.db3), &db);
> SVN_ERR(get_schema(version, &db, scratch_pool));
> SQLITE_ERR(sqlite3_close(db.db3), &db);
> @@ -509,15 +518,7 @@ svn_sqlite__open(svn_sqlite__db_t **db,
> int latest_schema, const char * const *upgrade_sql,
> apr_pool_t *result_pool, apr_pool_t *scratch_pool)
> {
> - static svn_boolean_t sqlite_initialized = FALSE;
> -
> - if (! sqlite_initialized)
> - {
> - /* There is a potential initialization race condition here, but
> - it currently isn't worth guarding against (e.g. with a
> mutex). */
> - SVN_ERR(init_sqlite());
> - sqlite_initialized = TRUE;
> - }
> + SVN_ERR(init_sqlite());
>
> /* ### use a pool cleanup to close this? (instead of __close()) */
> *db = apr_palloc(result_pool, sizeof(**db));
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageI
> d=1115288

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1118932
Received on 2009-02-07 13:55:49 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.