Revive D51728257 with fix (#1224)

Summary:

Revive the localtime() thread safety change which is reverted due to
crashes on Windows (D52210979). Fix is applied.

Differential Revision: D52213926

lib/Platform/Unicode/PlatformUnicodeCF.cpp

@@ -51,9 +51,10 @@ double localOffsetFromGMT() {
   }
 
   // Deconstruct the time into localTime.
-  std::tm *local = std::localtime(&currentWithDST);
+  std::tm tm;
+  std::tm *local = ::localtime_r(&currentWithDST, &tm);
   if (!local) {
-    llvm_unreachable("localtime failed in localOffsetFromGMT()");
+    return 0;
   }
 
   return local->tm_gmtoff * MS_PER_SECOND;

lib/VM/JSLib/DateUtil.cpp

@@ -218,26 +218,25 @@ double localTZA() {
 
   long gmtoff;
   int err = _get_timezone(&gmtoff);
-  assert(err == 0 && "_get_timezone failed in localTZA()");
+  if (err)
+    return 0;
 
   // The result of _get_timezone is negated
   return -gmtoff * MS_PER_SECOND;
 
 #else
 
-  ::tzset();
-
   // Get the current time in seconds (might have DST adjustment included).
-  time_t currentWithDST = std::time(nullptr);
-  if (currentWithDST == static_cast<time_t>(-1)) {
-    return 0;
-  }
+  std::time_t currentWithDST = std::time(nullptr);
 
   // Deconstruct the time into localTime.
-  std::tm *local = std::localtime(&currentWithDST);
-  if (!local) {
-    llvm_unreachable("localtime failed in localTZA()");
-  }
+  // Note that localtime_r uses cached timezone information on Linux (glibc) and
+  // Windows, so the returned local time may not be computed using an updated
+  // timezone.
+  std::tm tm;
+  std::tm *local = ::localtime_r(&currentWithDST, &tm);
+  if (!local)
+    return 0;
 
   long gmtoff = local->tm_gmtoff;
 
@@ -375,12 +374,11 @@ int32_t detail::equivalentTime(int64_t epochSecs) {
 }
 
 double daylightSavingTA(double t) {
+  // LocalTime should only take finite time value.
   if (!std::isfinite(t)) {
     return std::numeric_limits<double>::quiet_NaN();
   }
 
-  ::tzset();
-
   // Convert t to seconds and get the actual time needed.
   const double seconds = t / MS_PER_SECOND;
   // If the number of seconds is higher or lower than a unix timestamp can
@@ -397,12 +395,23 @@ double daylightSavingTA(double t) {
   // savings time calculations.
   local = detail::equivalentTime(static_cast<int64_t>(seconds));
 
-  std::tm *brokenTime = std::localtime(&local);
+  std::tm tm;
+#ifdef _WINDOWS
+  // The return value of localtime_s on Windows is an error code instead of
+  // a pointer to std::tm. For simplicity, we don't inspect the concrete error
+  // code and just return 0.
+  auto err = ::localtime_s(&tm, &local);
+  if (err) {
+    return 0;
+  }
+#else
+  std::tm *brokenTime = ::localtime_r(&local, &tm);
   if (!brokenTime) {
     // Local time is invalid.
-    return std::numeric_limits<double>::quiet_NaN();
+    return 0;
   }
-  return brokenTime->tm_isdst ? MS_PER_HOUR : 0;
+#endif
+  return tm.tm_isdst ? MS_PER_HOUR : 0;
 }
 
 //===----------------------------------------------------------------------===//

test/hermes/date-local-to-utc.js

@@ -6,6 +6,7 @@
  */
 
 // RUN: TZ="America/Santiago" %hermes -O %s | %FileCheck --match-full-lines %s
+// XFAIL: windows
 "use strict";
 
 // These tests ensure that local to UTC time conversion yields consistent result

unittests/VMRuntime/DateUtilTest.cpp

@@ -121,6 +121,16 @@ TEST(DateUtilTest, WeekDayTest) {
   EXPECT_EQ(3, weekDay(23415386789000)); // Wed, Jan 3, 2712
 }
 
+namespace {
+/// `localtime_r()` called in localTZA() caches the timezone. To correctly
+/// handle different timezones in a single test, let's call `tzset()` explicitly
+/// to update the cached timezone.
+void setTimeZone(const char *tzname) {
+  hermes::oscompat::set_env("TZ", tzname);
+  ::tzset();
+}
+} // namespace
+
 TEST(DateUtilTest, LocalTZATest) {
   // On Windows, TZ env can only be set to a very limited format,
   // as documented in Microsoft Docs for _tzset. Specifically,
@@ -135,17 +145,17 @@ TEST(DateUtilTest, LocalTZATest) {
 
   // US Pacific: DST is from Mar to Nov
 #ifdef _WINDOWS
-  hermes::oscompat::set_env("TZ", "PST8PDT");
+  setTimeZone("PST8PDT");
 #else
-  hermes::oscompat::set_env("TZ", "America/Los_Angeles");
+  setTimeZone("America/Los_Angeles");
 #endif
   EXPECT_EQ(-2.88e+7, localTZA());
 
   // New Zealand: DST is from Oct to Apr
 #ifdef _WINDOWS
   // This test is skipped due to Windows deficiency in TZ env variable.
 #else
-  hermes::oscompat::set_env("TZ", "Pacific/Auckland");
+  setTimeZone("Pacific/Auckland");
   EXPECT_EQ(4.32e+7, localTZA());
 #endif
 
@@ -154,17 +164,17 @@ TEST(DateUtilTest, LocalTZATest) {
 
   // Negative fixed zone
 #ifdef _WINDOWS
-  hermes::oscompat::set_env("TZ", "PST8");
+  setTimeZone("PST8");
 #else
-  hermes::oscompat::set_env("TZ", "Etc/GMT+8");
+  setTimeZone("Etc/GMT+8");
 #endif
   EXPECT_EQ(-2.88e+7, localTZA());
 
   // Positive fixed zone
 #ifdef _WINDOWS
-  hermes::oscompat::set_env("TZ", "JST-9");
+  setTimeZone("JST-9");
 #else
-  hermes::oscompat::set_env("TZ", "Asia/Tokyo");
+  setTimeZone("Asia/Tokyo");
 #endif
   EXPECT_EQ(3.24e+7, localTZA());
 
@@ -202,13 +212,13 @@ TEST(DateUtilTest, EquivalentTimeTest) {
 }
 
 TEST(DateUtilTest, DaylightSavingTATest) {
-  hermes::oscompat::set_env("TZ", "America/Los_Angeles");
+  setTimeZone("America/Los_Angeles");
   EXPECT_EQ(MS_PER_HOUR, daylightSavingTA(1489530532000)); // Mar 14, 2017
   EXPECT_EQ(MS_PER_HOUR, daylightSavingTA(1019514530000)); // Apr 22, 2002
   EXPECT_EQ(0, daylightSavingTA(1487111330000)); // Feb 14, 2017
   EXPECT_EQ(0, daylightSavingTA(1017700130000)); // Apr 1, 2002
 
-  hermes::oscompat::set_env("TZ", "America/Chicago");
+  setTimeZone("America/Chicago");
   EXPECT_EQ(MS_PER_HOUR, daylightSavingTA(1489530532000)); // Mar 14, 2017
   EXPECT_EQ(MS_PER_HOUR, daylightSavingTA(1019514530000)); // Apr 22, 2002
   EXPECT_EQ(0, daylightSavingTA(1487111330000)); // Feb 14, 2017
@@ -234,33 +244,33 @@ TEST(DateUtilTest, LocalTimeTest) {
   // 2018-07-02T01:00:00+0900[Asia/Tokyo] (DST not observed)
 
 #ifdef _WINDOWS
-  hermes::oscompat::set_env("TZ", "PST8PDT");
+  setTimeZone("PST8PDT");
 #else
-  hermes::oscompat::set_env("TZ", "America/Los_Angeles");
+  setTimeZone("America/Los_Angeles");
 #endif
   EXPECT_EQ(1530435600000, localTime(1530460800000));
   EXPECT_EQ(1530460800000, utcTime(1530435600000));
 
 #ifdef _WINDOWS
-  hermes::oscompat::set_env("TZ", "EST5EDT");
+  setTimeZone("EST5EDT");
 #else
-  hermes::oscompat::set_env("TZ", "America/New_York");
+  setTimeZone("America/New_York");
 #endif
   EXPECT_EQ(1530446400000, localTime(1530460800000));
   EXPECT_EQ(1530460800000, utcTime(1530446400000));
 
 #ifdef _WINDOWS
   // This test is skipped due to Windows deficiency in TZ env variable.
 #else
-  hermes::oscompat::set_env("TZ", "Pacific/Auckland");
+  setTimeZone("Pacific/Auckland");
   EXPECT_EQ(1530504000000, localTime(1530460800000));
   EXPECT_EQ(1530460800000, utcTime(1530504000000));
 #endif
 
 #ifdef _WINDOWS
-  hermes::oscompat::set_env("TZ", "JST-9");
+  setTimeZone("JST-9");
 #else
-  hermes::oscompat::set_env("TZ", "Asia/Tokyo");
+  setTimeZone("Asia/Tokyo");
 #endif
   EXPECT_EQ(1530493200000, localTime(1530460800000));
   EXPECT_EQ(1530460800000, utcTime(1530493200000));