850300 - Helgrind warnings about data races with gmtime, localtime when gmtime_r/localtime_r are unavailable

Status: RESOLVED INCOMPLETE

(Core :: JavaScript Engine, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

==3013== Possible data race during read of size 4 at 0xB6F2F20 by thread #10
==3013== Locks held: none
==3013==    at 0x8DAA69D: ComputeUTCTime(long, tm*) (DateTime.cpp:36)
==3013==    by 0x8DAA75B: UTCToLocalStandardOffsetSeconds() (DateTime.cpp:104)
==3013==    by 0x8DAA860: js::DateTimeInfo::updateTimeZoneAdjustment() (DateTime.cpp:130)
==3013==    by 0x8DAABAA: js::DateTimeInfo::DateTimeInfo() (DateTime.cpp:160)
==3013==    by 0x89989BA: JSRuntime::JSRuntime(JSUseHelperThreads) (jsapi.cpp:868)
==3013==    by 0x89BA12C: JS_NewRuntime(unsigned int, JSUseHelperThreads) (Utility.h:504)
==3013==    by 0x6792317: (anonymous namespace)::CreateJSContextForWorker(mozilla::dom::workers::WorkerPrivate*) (RuntimeService.cpp:420)
==3013==    by 0x67925F5: (anonymous namespace)::WorkerThreadRunnable::Run() (RuntimeService.cpp:503)
==3013==    by 0x7D328B5: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:637)
==3013==    by 0x7CB884C: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:238)
==3013==    by 0x7D315FB: nsThread::ThreadFunc(void*) (nsThread.cpp:269)
==3013==    by 0x4097624: _pt_root (ptthread.c:192)
==3013==    by 0x403032F: mythread_wrapper (hg_intercepts.c:219)
==3013==    by 0x4A2CE99: start_thread (pthread_create.c:308)
==3013==    by 0xB428CBC: clone (clone.S:112)
==3013== 
==3013== This conflicts with a previous write of size 4 by thread #9
==3013== Locks held: none
==3013==    at 0xB3E65B7: __tz_convert (tzset.c:643)
==3013==    by 0x8DAA664: ComputeUTCTime(long, tm*) (DateTime.cpp:33)
==3013==    by 0x8DAA75B: UTCToLocalStandardOffsetSeconds() (DateTime.cpp:104)
==3013==    by 0x8DAA860: js::DateTimeInfo::updateTimeZoneAdjustment() (DateTime.cpp:130)
==3013==    by 0x8DAABAA: js::DateTimeInfo::DateTimeInfo() (DateTime.cpp:160)
==3013==    by 0x89989BA: JSRuntime::JSRuntime(JSUseHelperThreads) (jsapi.cpp:868)
==3013==    by 0x89BA12C: JS_NewRuntime(unsigned int, JSUseHelperThreads) (Utility.h:504)
==3013==    by 0x6792317: (anonymous namespace)::CreateJSContextForWorker(mozilla::dom::workers::WorkerPrivate*) (RuntimeService.cpp:420)
==3013==    by 0x67925F5: (anonymous namespace)::WorkerThreadRunnable::Run() (RuntimeService.cpp:503)
==3013==    by 0x7D328B5: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:637)
==3013==    by 0x7CB884C: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:238)
==3013==    by 0x7D315FB: nsThread::ThreadFunc(void*) (nsThread.cpp:269)
==3013==    by 0x4097624: _pt_root (ptthread.c:192)
==3013==    by 0x403032F: mythread_wrapper (hg_intercepts.c:219)
==3013==    by 0x4A2CE99: start_thread (pthread_create.c:308)
==3013==    by 0xB428CBC: clone (clone.S:112)

I'm not sure how important this is, but the docs I looked at [1] say this:

  The function also accesses and modifies a shared internal object,
  which may introduce data races on concurrent calls to gmtime and
  localtime. Some libraries provide an alternative function that avoids
  this data race: gmtime_r (non-portable).

[1]: http://www.cplusplus.com/reference/ctime/gmtime

Comment 1

[Jeff Walden [:Waldo]]

Take a closer look at ComputeUTCTime: if gmtime_r is available, we use it.  Does your system not provide gmtime_r?  I'm not sure how we can reasonably work around that, other than by racing as we are here.

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #1)
> Take a closer look at ComputeUTCTime: if gmtime_r is available, we use it.

Yeah, I saw that, but that doesn't help in all cases. That's why I filed this bug :)

> Does your system not provide gmtime_r?

Apparently not. I'm on ubuntu 64 using gcc 4.6, nothing special.

Comment 3

[Jeff Walden [:Waldo]]

I would consider systems without gmtime_r "special".  :-P

There's almost certainly an identical race for localtime/localtime_r with ComputeLocalTime.

Comment 5

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #3)
> I would consider systems without gmtime_r "special".

Actually I have gmtime_r on my system. Looks like our configure test for this is broken.

Comment 6

[Jeff Walden [:Waldo]]

The configure test WFM locally, or at least breaking on ComputeLocalTime and such go into the _r versions.  So something truly odd is going on here.