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

Re: Thread-safe error location in maintainer mode

From: Branko Čibej <brane_at_wandisco.com>
Date: Wed, 04 Mar 2015 22:45:36 +0100

On 04.03.2015 17:04, Philip Martin wrote:
> Branko Čibej <brane_at_wandisco.com> writes:
>
>> And here it is: the whole atomic.c file
>>
>> https://paste.apache.org/2iyN
>>
>> and the diff against current trunk:
>>
>> https://paste.apache.org/cK1R
>>
> I prefer patches to be included in the email. Ideally inline, as far as
> I am concerned, or as an attachment.

In this case, looking at the whole file is actually easier to review ...
anyway, here's the latest version of the patch:

Index: subversion/libsvn_subr/atomic.c
===================================================================
--- subversion/libsvn_subr/atomic.c (revision 1663988)
+++ subversion/libsvn_subr/atomic.c (working copy)
@@ -42,44 +42,53 @@
   /* We have to call init_func exactly once. Because APR
      doesn't have statically-initialized mutexes, we implement a poor
      man's spinlock using svn_atomic_cas. */
+
+ svn_error_t *err = SVN_NO_ERROR;
   svn_atomic_t status = svn_atomic_cas(global_status,
                                        SVN_ATOMIC_START_INIT,
                                        SVN_ATOMIC_UNINITIALIZED);
 
- if (status == SVN_ATOMIC_UNINITIALIZED)
+ for (;;)
     {
- svn_error_t *err = init_func(baton, pool);
- if (err)
+ switch (status)
         {
-#if APR_HAS_THREADS
- /* Tell other threads that the initialization failed. */
- svn_atomic_cas(global_status,
- SVN_ATOMIC_INIT_FAILED,
- SVN_ATOMIC_START_INIT);
-#endif
+ case SVN_ATOMIC_UNINITIALIZED:
+ err = init_func(baton, pool);
+ if (err)
+ {
+ status = svn_atomic_cas(global_status,
+ SVN_ATOMIC_INIT_FAILED,
+ SVN_ATOMIC_START_INIT);
+ }
+ else
+ {
+ status = svn_atomic_cas(global_status,
+ SVN_ATOMIC_INITIALIZED,
+ SVN_ATOMIC_START_INIT);
+ }
+
+ /* Take another spin through the switch to make sure that we
+ have just set a valid value. */
+ continue;
+
+ case SVN_ATOMIC_START_INIT:
+ /* Wait for the init function to complete. */
+ apr_sleep(APR_USEC_PER_SEC / 1000);
+ status = svn_atomic_cas(global_status,
+ SVN_ATOMIC_UNINITIALIZED,
+ SVN_ATOMIC_UNINITIALIZED);
+ continue;
+
+ case SVN_ATOMIC_INIT_FAILED:
           return svn_error_create(SVN_ERR_ATOMIC_INIT_FAILURE, err,
                                   "Couldn't perform atomic initialization");
+
+ case SVN_ATOMIC_INITIALIZED:
+ return SVN_NO_ERROR;
+
+ default:
+ /* Something went seriously wrong with the atomic operations. */
+ abort();
         }
- svn_atomic_cas(global_status,
- SVN_ATOMIC_INITIALIZED,
- SVN_ATOMIC_START_INIT);
     }
-#if APR_HAS_THREADS
- /* Wait for whichever thread is performing initialization to finish. */
- /* XXX FIXME: Should we have a maximum wait here, like we have in
- the Windows file IO spinner? */
- else while (status != SVN_ATOMIC_INITIALIZED)
- {
- if (status == SVN_ATOMIC_INIT_FAILED)
- return svn_error_create(SVN_ERR_ATOMIC_INIT_FAILURE, NULL,
- "Couldn't perform atomic initialization");
-
- apr_sleep(APR_USEC_PER_SEC / 1000);
- status = svn_atomic_cas(global_status,
- SVN_ATOMIC_UNINITIALIZED,
- SVN_ATOMIC_UNINITIALIZED);
- }
-#endif /* APR_HAS_THREADS */
-
- return SVN_NO_ERROR;
 }
Received on 2015-03-04 22:46:58 CET

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