On 9/3/05, Xavier Leroy <Xavier.Leroy@inria.fr> wrote:
> > First off, the underlying localtime call is definitely not re-entrant.
> > The tm data structure is shared among all calls, leading to the
> > possibility of races.
> 
> As others mentioned, the C library implementation of this function
> could use thread-local storage to avoid this particular race.  I don't
> think this is guaranteed, though.

Indeed, the man page explicitly states that the function is not
threadsafe.  I think this counts as a bug, and I suspect I may have
seen the bug in the wild.  It seems like it would be simple and safe
to use localtime_r instead.  Should I submit a patch?

> > If I understand the way ocaml's locking works, another thread could
> > make a call to localtime where alloc_small is called.
> 
> No, at least not another Caml thread: GC triggered from C code
> (e.g. alloc_small()) cannot cause context switches.  So, from a Caml
> standpoint, the call to localtime() and the following allocation of
> its result are atomic.
> 
> > If there are mixed C threads and Ocaml threads, then a call by the C
> > thread at that point could muck up the tm data structure before
> > switching back to OCaml, thus leading to bad data getting into the
> > tm data structure.
> 
> Yes, this is possible in principle if 1- some non-Caml threads call
> localtime(), and 2- the implementation of that function does not use
> thread-local storage.

It's interesting to learn that context switches can't happen on c-land
allocations.  That simplifies a lot of things.  But the C-interactions
are still possible, so I would still argue for a fix.  I'm still quite
unsure if the bug I saw is a result of this race, and realizing that
the race can't be between caml-threads makes me more unsure.

Thanks for the help,
Yaron