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

Re: svn commit: r1004286 - in /subversion/trunk: ./ subversion/libsvn_subr/io.c

From: Ramkumar Ramachandra <artagnon_at_gmail.com>
Date: Mon, 11 Oct 2010 18:24:07 +0530

Hi Bert and Julian,

Bert Huijben writes:
> > -----Original Message-----
> > From: Julian Foad [mailto:julian.foad_at_wandisco.com]
> > On Mon, 2010-10-11, Julian Foad wrote:
> > > <http://stackoverflow.com/questions/54188/are-c-reads-and-writes-of-
> > an-int-atomic>.
> > > "On an IA32 a correctly aligned address will be an atomic operation.
> > > [... otherwise... can't assume it is]."
> >
> > Sorry, I pressed "Send" too early. That's not the most important bit of
> > information. (That paragraph talks mostly about <= 32-bit CPUs, where
> > of course there will be problems.) Bert explained to me on IRC that
> > atomicity is not guaranteed even on >= 32 bit architectures, and the
> > highest-ranked answer on that web page agrees. I'm no expert in this so
> > I'll go away now.
>
> Let me add that calling apr_atomic_set32() instead of the 'x = <value>' part of the pattern will fix this issue in the way that was documented by the comment in the code: all threads do the same thing and one of the results is left in the static variable.
>
> Another option is to use an atomic initialization function to initialize the value just once.

I see; I don't think I know enough to comment.
Bert: So does this solve the issue or did I misunderstand something?

Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 1005706)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -1269,7 +1269,8 @@ static svn_error_t *
 get_default_file_perms(apr_fileperms_t *perms, apr_pool_t *scratch_pool)
 {
   /* the default permissions as read from the temp folder */
- static apr_fileperms_t default_perms = 0;
+ static apr_fileperms_t default_perms;
+ apr_atomic_set32(&default_perms, 0);
 
   /* Technically, this "racy": Multiple threads may use enter here and
      try to figure out the default permisission concurrently. That's fine
@@ -1303,7 +1304,7 @@ get_default_file_perms(apr_fileperms_t *perms, apr
       SVN_ERR(svn_io_file_close(fd, scratch_pool));
 
       *perms = finfo.protection;
- default_perms = finfo.protection;
+ apr_atomic_set32(&default_perms, finfo.protection);
     }
   else
     *perms = default_perms;
Received on 2010-10-11 14:55:46 CEST

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.