On Mon, 2010-10-11, Julian Foad wrote:
> On Mon, 2010-10-11, Bert Huijben wrote:
> > > -----Original Message-----
> > > From: Greg Stein [mailto:gstein_at_gmail.com]
> > > On Mon, Oct 11, 2010 at 04:58, Bert Huijben <bert_at_qqmail.nl> wrote:
> > > >> -----Original Message-----
> > > >> From: artagnon_at_apache.org [mailto:artagnon_at_apache.org]
> > > >> * subversion/libsvn_subr/io.c
> > > >> (get_default_file_perms): Store the permissions of the created
> > > >> temporary file in a static variable and re-use it in subsequent
> > > >> calls instead of checking persmissions everytime. This has
> > > >> performance benefits.
> > > >
> > > > Shouldn't this function use some 'atomic initialization' handling?
> > > >
> > > > Currently it uses a static apr_fileperms_t (integer?) which can be
> > > manipulated by multiple threads at the same time.
> > > >
> > > > This part of subversion is a library and inside tools like Subclipse,
> > > TortoiseSVN, AnkhSVN and others it is used multithreaded.
> > >
> > > So what? Aren't all of those threads going to write the exact same
> > > value into the variable?
> > >
> > > And if they *don't*, then I believe we have larger problems.
> >
> > No, they are taking the value that is being stored there at the same time
> > (read: value is undefined) and use that as the umask.
> >
> > The pattern
> > static long x;
>
> > if (x == 0)
> > {
> > <calculating>
> > x = <value>
> > }
> > <use x>
> >
> > is not thread safe.
> <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.
- Julian
Received on 2010-10-11 11:46:19 CEST