On Tue, 8 Apr 2025, Corinna Vinschen wrote:
> On Apr 8 10:26, Jeremy Drake via Cygwin-patches wrote:
> > On Sun, 6 Apr 2025, Takashi Yano wrote:
> >
> > > Revised.
> >
> > Sorry to be late to the party (I didn't open the attachment before, and
> > only saw the commit):
> >
> > > + static class keys_list {
> > > + LONG64 seq;
> > > + LONG64 busy_cnt;
> >
> > Should these be `volatile`? busy_cnt is probably OK, it only seems to be
> > dealt with via Interlocked functions, but seq is tested directly in some
> > cases and via Interlocked in others.
>
> One could think so, but...
>
> ....GLibc uses (basically) the same mechanism. It reads seq in
> pthread_key_create and pthread_key_delete without atomic access, and
> neither the global array keeping the key info, nor the seq struct member
> in there are marked volatile.
>
> Is there anything which makes our version different?
I didn't realize that this was based on glibc code. I don't know if they
know something I don't, or this is an oversight. But it's probably OK
regardless, I don't think the compiler can know anything in practice that
could result in it skipping a read of those variables.
