Bert Huijben wrote:
>> -----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.
Yeah, seeing that (and some other) comments in this file gave me the willies,
too. I'm planning on doing the appropriate atomization wrapping in a bit.
>> 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=462&dsMessageId=1120580
Received on 2009-02-07 21:04:28 CET