Add lock to prevent race on SysTimeFromTimeStruct

This change preents a race on the 'TZ' environ global variable in libc.
This should prevent a crash when base::Time::LocalMidnight calls out to
Time::FromExploded() which invokes mktime(), which relies on the timezone,
which is initialized when initializing libc (bionic on Android).

BUG=390567
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288544

[bengr, 2014-08-04 22:38:00]

PTAL

[willchan no longer on Chromium, 2014-08-05 23:14:48]

https://codereview.chromium.org/444473002/diff/1/base/time/time_posix.cc
File base/time/time_posix.cc (right):

https://codereview.chromium.org/444473002/diff/1/base/time/time_posix.cc#newc...
base/time/time_posix.cc:33: #if defined(OS_ANDROID)
No need for the OS_ANDROID stuff. This bug isn't Android specific, it applies to
all posix implementations doing an unsynchronized read/write on the TZ global
environment variable.

[bengr, 2014-08-05 23:54:58]

https://codereview.chromium.org/444473002/diff/1/base/time/time_posix.cc
File base/time/time_posix.cc (right):

https://codereview.chromium.org/444473002/diff/1/base/time/time_posix.cc#newc...
base/time/time_posix.cc:33: #if defined(OS_ANDROID)
On 2014/08/05 23:14:48, willchan wrote:
> No need for the OS_ANDROID stuff. This bug isn't Android specific, it applies
to
> all posix implementations doing an unsynchronized read/write on the TZ global
> environment variable.

Done.

[willchan no longer on Chromium, 2014-08-06 20:43:24]

LGTM, I hope this fixes the bug reports. Seems possible.

https://codereview.chromium.org/444473002/diff/40001/base/time/time_posix.cc
File base/time/time_posix.cc (right):

https://codereview.chromium.org/444473002/diff/40001/base/time/time_posix.cc#...
base/time/time_posix.cc:34: // the 'TZ' variable in libc.
Please update this to reference the bug so people can understand what's going
on.

[bengr, 2014-08-07 18:28:30]

https://codereview.chromium.org/444473002/diff/40001/base/time/time_posix.cc
File base/time/time_posix.cc (right):

https://codereview.chromium.org/444473002/diff/40001/base/time/time_posix.cc#...
base/time/time_posix.cc:34: // the 'TZ' variable in libc.
On 2014/08/06 20:43:24, willchan wrote:
> Please update this to reference the bug so people can understand what's going
> on.

Done.

[bengr, 2014-08-07 18:28:34]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-08-07 18:33:12]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bengr@chromium.org/444473002/60001

[commit-bot: I haz the power, 2014-08-07 23:21:00]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/38769)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[bengr, 2014-08-08 00:46:25]

On 2014/08/07 23:21:00, I haz the power (commit-bot) wrote:
> FYI, CQ is re-trying this CL (attempt #1).
> The failing builders are:
>   mac_gpu on tryserver.chromium.gpu
>
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/38769)
>   ios_dbg_simulator on tryserver.chromium.mac
>
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
>   ios_rel_device on tryserver.chromium.mac
>
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
>   ios_rel_device_ninja on tryserver.chromium.mac
>
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
>   mac_chromium_compile_dbg on tryserver.chromium.mac
>
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
>   mac_chromium_rel_swarming on tryserver.chromium.mac
>
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

willchan@, do you know what's going on? We could limit this fix to Android.

[willchan no longer on Chromium, 2014-08-08 00:49:38]

I see, the compile error says the LazyInstance isn't used at all on Mac.
Move that LazyInstance into the #if !defined(OS_MACOSX) block.


On Thu, Aug 7, 2014 at 5:46 PM, <bengr@chromium.org> wrote:

> On 2014/08/07 23:21:00, I haz the power (commit-bot) wrote:
>
>> FYI, CQ is re-trying this CL (attempt #1).
>> The failing builders are:
>>    mac_gpu on tryserver.chromium.gpu
>>
>
> (http://build.chromium.org/p/tryserver.chromium.gpu/
> builders/mac_gpu/builds/38769)
>
>>    ios_dbg_simulator on tryserver.chromium.mac
>>
>
> (http://build.chromium.org/p/tryserver.chromium.mac/
> builders/ios_dbg_simulator/builds/3564)
>
>>    ios_rel_device on tryserver.chromium.mac
>>
>
> (http://build.chromium.org/p/tryserver.chromium.mac/
> builders/ios_rel_device/builds/3568)
>
>>    ios_rel_device_ninja on tryserver.chromium.mac
>>
>
> (http://build.chromium.org/p/tryserver.chromium.mac/
> builders/ios_rel_device_ninja/builds/2456)
>
>>    mac_chromium_compile_dbg on tryserver.chromium.mac
>>
>
> (http://build.chromium.org/p/tryserver.chromium.mac/
> builders/mac_chromium_compile_dbg/builds/3840)
>
>>    mac_chromium_rel_swarming on tryserver.chromium.mac
>>
>
> (http://build.chromium.org/p/tryserver.chromium.mac/
> builders/mac_chromium_rel_swarming/builds/876)
>
> willchan@, do you know what's going on? We could limit this fix to
> Android.
>
> https://codereview.chromium.org/444473002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2014-08-08 01:03:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-08 01:03:17]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/38871)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[bengr, 2014-08-09 00:42:08]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-08-09 00:44:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bengr@chromium.org/444473002/80001

[commit-bot: I haz the power, 2014-08-09 07:32:27]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-09 08:50:48]

Change committed as 288544