Re: [Caml-list] Re: Unix.localtime not threadsafe?

Re: [Caml-list] Re: Unix.localtime not threadsafe?

[Damien Bobillot]

[-- Attachment #1: Type: text/plain, Size: 583 bytes --]


Florian Weimer wrote :


> * Bardur Arantsson:
>
>
>> I don't think the glibc/Linux localtime() man page explicitly states
>> this, but I expect that it returns a pointer to a *thread-local*
>> statically allocated struct tm... in which case there's no problem.
>>
>
> Thread-local storage is a recent innovation in the Linux camp.
> Previously, developers seem to think that no efficient implementation
> was possible.  That's why there all the *_r variants (such as
> localtime_r).
>

For information : Mac OS X use thread-local storage since version 10.2.

-- 
Damien Bobillot



[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2375 bytes --]

Unix.localtime not threadsafe?

[Yaron Minsky]

I was looking at the Unix.localtime implementation, and it appears to
me that it's not threadsafe.  I'm wondering if anyone can confirm.

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.  What I'm not sure of is whether there can be a
race given the locking of the OCaml runtime.  Here's the code from
gmtime.c in the ocaml distribution:

static value alloc_tm(struct tm *tm)
{
  value res;
  res = alloc_small(9, 0);
  Field(res,0) = Val_int(tm->tm_sec);
  Field(res,1) = Val_int(tm->tm_min);
  Field(res,2) = Val_int(tm->tm_hour);
  Field(res,3) = Val_int(tm->tm_mday);
  Field(res,4) = Val_int(tm->tm_mon);
  Field(res,5) = Val_int(tm->tm_year);
  Field(res,6) = Val_int(tm->tm_wday);
  Field(res,7) = Val_int(tm->tm_yday);
  Field(res,8) = tm->tm_isdst ? Val_true : Val_false;
  return res;
}

CAMLprim value unix_localtime(value t)
{
  time_t clock;
  struct tm * tm;
  clock = (time_t) Double_val(t);
  tm = localtime(&clock);
  if (tm == NULL) unix_error(EINVAL, "localtime", Nothing);
  return alloc_tm(tm);
}

If I understand the way ocaml's locking works, another thread could
make a call to localtime where alloc_small is called.  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.

So, does this seem like a going bug?  We think we may have encountered
this in the wild, so this may be more than a theoretical bug.

Yaron

Re: Unix.localtime not threadsafe?

[Bardur Arantsson]

Yaron Minsky wrote:
> I was looking at the Unix.localtime implementation, and it appears to
> me that it's not threadsafe.  I'm wondering if anyone can confirm.
> 
> 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.  What I'm not sure of is whether there can be a
> race given the locking of the OCaml runtime.  Here's the code from
> gmtime.c in the ocaml distribution:

I don't think the glibc/Linux localtime() man page explicitly states 
this, but I expect that it returns a pointer to a *thread-local* 
statically allocated struct tm... in which case there's no problem.

Most other system functions whose API looks non-threadsafe do the same. 
('errno' would be the standard example).

[--snip--]

-- 
Bardur Arantsson
<bardur@imada.sdu.dk>
<bardur@scientician.net>

- But don't be reading my mind between 4 and 5. That's Willie's
time!
                              Groundskeeper Willie, 'The Simpsons'

Re: [Caml-list] Re: Unix.localtime not threadsafe?

[Florian Weimer]

* Bardur Arantsson:

> I don't think the glibc/Linux localtime() man page explicitly states 
> this, but I expect that it returns a pointer to a *thread-local* 
> statically allocated struct tm... in which case there's no problem.

Thread-local storage is a recent innovation in the Linux camp.
Previously, developers seem to think that no efficient implementation
was possible.  That's why there all the *_r variants (such as
localtime_r).

I believe Solaris does something in the direction you suggest.