From: Andy Heninger Date: Tue, 18 Sep 2018 20:04:15 +0000 (-0700) Subject: ICU-20104 Fix lazy init in Indian Calendar. Now matches other calendars. (#108) X-Git-Tag: release-63-rc~51 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f90676c2bc2fb801cd9c3f5d297329d85457f1f6;p=icu ICU-20104 Fix lazy init in Indian Calendar. Now matches other calendars. (#108) * ICU-20104 Fix lazy init in Indian Calendar. Now matches other calendars. * ICU-20104 export class IndianCalendar for testing. * ICU-20104 shot-in-the-dark Windows build experiment. * ICU-20104 fix memory leak in added test. --- diff --git a/icu4c/source/i18n/indiancal.cpp b/icu4c/source/i18n/indiancal.cpp index 278f9efb2e0..ae1098f0558 100644 --- a/icu4c/source/i18n/indiancal.cpp +++ b/icu4c/source/i18n/indiancal.cpp @@ -347,12 +347,15 @@ IndianCalendar::inDaylightTime(UErrorCode& status) const return (UBool)(U_SUCCESS(status) ? (internalGet(UCAL_DST_OFFSET) != 0) : FALSE); } -// default century -const UDate IndianCalendar::fgSystemDefaultCentury = DBL_MIN; -const int32_t IndianCalendar::fgSystemDefaultCenturyYear = -1; -UDate IndianCalendar::fgSystemDefaultCenturyStart = DBL_MIN; -int32_t IndianCalendar::fgSystemDefaultCenturyStartYear = -1; +/** + * The system maintains a static default century start date and Year. They are + * initialized the first time they are used. Once the system default century date + * and year are set, they do not change. + */ +static UDate gSystemDefaultCenturyStart = DBL_MIN; +static int32_t gSystemDefaultCenturyStartYear = -1; +static icu::UInitOnce gSystemDefaultCenturyInit = U_INITONCE_INITIALIZER; UBool IndianCalendar::haveDefaultCentury() const @@ -360,87 +363,45 @@ UBool IndianCalendar::haveDefaultCentury() const return TRUE; } -UDate IndianCalendar::defaultCenturyStart() const +static void U_CALLCONV +initializeSystemDefaultCentury() { - return internalGetDefaultCenturyStart(); -} + // initialize systemDefaultCentury and systemDefaultCenturyYear based + // on the current time. They'll be set to 80 years before + // the current time. + UErrorCode status = U_ZERO_ERROR; -int32_t IndianCalendar::defaultCenturyStartYear() const -{ - return internalGetDefaultCenturyStartYear(); -} + IndianCalendar calendar ( Locale ( "@calendar=Indian" ), status); + if ( U_SUCCESS ( status ) ) { + calendar.setTime ( Calendar::getNow(), status ); + calendar.add ( UCAL_YEAR, -80, status ); -UDate -IndianCalendar::internalGetDefaultCenturyStart() const -{ - // lazy-evaluate systemDefaultCenturyStart - UBool needsUpdate; - { - Mutex m; - needsUpdate = (fgSystemDefaultCenturyStart == fgSystemDefaultCentury); - } + UDate newStart = calendar.getTime ( status ); + int32_t newYear = calendar.get ( UCAL_YEAR, status ); - if (needsUpdate) { - initializeSystemDefaultCentury(); + gSystemDefaultCenturyStart = newStart; + gSystemDefaultCenturyStartYear = newYear; } + // We have no recourse upon failure. +} - // use defaultCenturyStart unless it's the flag value; - // then use systemDefaultCenturyStart - return fgSystemDefaultCenturyStart; +UDate +IndianCalendar::defaultCenturyStart() const +{ + // lazy-evaluate systemDefaultCenturyStart + umtx_initOnce(gSystemDefaultCenturyInit, &initializeSystemDefaultCentury); + return gSystemDefaultCenturyStart; } int32_t -IndianCalendar::internalGetDefaultCenturyStartYear() const +IndianCalendar::defaultCenturyStartYear() const { // lazy-evaluate systemDefaultCenturyStartYear - UBool needsUpdate; - { - Mutex m; - - needsUpdate = (fgSystemDefaultCenturyStart == fgSystemDefaultCentury); - } - - if (needsUpdate) { - initializeSystemDefaultCentury(); - } - - // use defaultCenturyStart unless it's the flag value; - // then use systemDefaultCenturyStartYear - - return fgSystemDefaultCenturyStartYear; + umtx_initOnce(gSystemDefaultCenturyInit, &initializeSystemDefaultCentury); + return gSystemDefaultCenturyStartYear; } -void -IndianCalendar::initializeSystemDefaultCentury() -{ - // initialize systemDefaultCentury and systemDefaultCenturyYear based - // on the current time. They'll be set to 80 years before - // the current time. - // No point in locking as it should be idempotent. - if (fgSystemDefaultCenturyStart == fgSystemDefaultCentury) { - UErrorCode status = U_ZERO_ERROR; - - IndianCalendar calendar(Locale("@calendar=Indian"),status); - if (U_SUCCESS(status)) { - calendar.setTime(Calendar::getNow(), status); - calendar.add(UCAL_YEAR, -80, status); - - UDate newStart = calendar.getTime(status); - int32_t newYear = calendar.get(UCAL_YEAR, status); - - { - Mutex m; - - fgSystemDefaultCenturyStart = newStart; - fgSystemDefaultCenturyStartYear = newYear; - } - } - - // We have no recourse upon failure unless we want to propagate the failure - // out. - } -} UOBJECT_DEFINE_RTTI_IMPLEMENTATION(IndianCalendar) diff --git a/icu4c/source/i18n/indiancal.h b/icu4c/source/i18n/indiancal.h index b5e0f963d95..e36d4eb5c0e 100644 --- a/icu4c/source/i18n/indiancal.h +++ b/icu4c/source/i18n/indiancal.h @@ -68,7 +68,7 @@ U_NAMESPACE_BEGIN */ -class IndianCalendar : public Calendar { +class U_I18N_API IndianCalendar : public Calendar { public: /** * Useful constants for IndianCalendar. @@ -274,10 +274,10 @@ public: * @return The class ID for all objects of this class. * @internal */ - U_I18N_API static UClassID U_EXPORT2 getStaticClassID(void); + static UClassID U_EXPORT2 getStaticClassID(void); /** - * return the calendar type, "buddhist". + * return the calendar type, "indian". * * @return calendar type * @internal @@ -320,49 +320,6 @@ protected: * @internal */ virtual int32_t defaultCenturyStartYear() const; - - private: // default century stuff. - /** - * The system maintains a static default century start date. This is initialized - * the first time it is used. Before then, it is set to SYSTEM_DEFAULT_CENTURY to - * indicate an uninitialized state. Once the system default century date and year - * are set, they do not change. - */ - static UDate fgSystemDefaultCenturyStart; - - /** - * See documentation for systemDefaultCenturyStart. - */ - static int32_t fgSystemDefaultCenturyStartYear; - - /** - * Default value that indicates the defaultCenturyStartYear is unitialized - */ - static const int32_t fgSystemDefaultCenturyYear; - - /** - * start of default century, as a date - */ - static const UDate fgSystemDefaultCentury; - - /** - * Returns the beginning date of the 100-year window that dates - * with 2-digit years are considered to fall within. - */ - UDate internalGetDefaultCenturyStart(void) const; - - /** - * Returns the first year of the 100-year window that dates with - * 2-digit years are considered to fall within. - */ - int32_t internalGetDefaultCenturyStartYear(void) const; - - /** - * Initializes the 100-year window that dates with 2-digit years - * are considered to fall within so that its start date is 80 years - * before the current time. - */ - static void initializeSystemDefaultCentury(void); }; U_NAMESPACE_END diff --git a/icu4c/source/test/intltest/tsmthred.cpp b/icu4c/source/test/intltest/tsmthred.cpp index eb7b882f5a0..7615637d885 100644 --- a/icu4c/source/test/intltest/tsmthred.cpp +++ b/icu4c/source/test/intltest/tsmthred.cpp @@ -13,6 +13,7 @@ #include "umutex.h" #include "cmemory.h" #include "cstring.h" +#include "indiancal.h" #include "uparse.h" #include "unicode/localpointer.h" #include "unicode/resbund.h" @@ -82,6 +83,7 @@ void MultithreadTest::runIndexedTest( int32_t index, UBool exec, TESTCASE_AUTO(TestBreakTranslit); TESTCASE_AUTO(TestIncDec); #endif /* #if !UCONFIG_NO_TRANSLITERATION */ + TESTCASE_AUTO(Test20104); TESTCASE_AUTO_END } @@ -1549,4 +1551,35 @@ void MultithreadTest::TestIncDec() } +static Calendar *gSharedCalendar = {}; + +class Test20104Thread : public SimpleThread { +public: + Test20104Thread() { }; + virtual void run(); +}; + +void Test20104Thread::run() { + gSharedCalendar->defaultCenturyStartYear(); +} + +void MultithreadTest::Test20104() { + UErrorCode status = U_ZERO_ERROR; + Locale loc("hi_IN"); + gSharedCalendar = new IndianCalendar(loc, status); + assertSuccess("Test20104", status); + + static constexpr int NUM_THREADS = 4; + Test20104Thread threads[NUM_THREADS]; + for (auto &thread:threads) { + thread.start(); + } + for (auto &thread:threads) { + thread.join(); + } + delete gSharedCalendar; + // Note: failure is reported by Thread Sanitizer. Test itself succeeds. +} + + #endif /* !UCONFIG_NO_TRANSLITERATION */ diff --git a/icu4c/source/test/intltest/tsmthred.h b/icu4c/source/test/intltest/tsmthred.h index b2517e22203..565080ac69a 100644 --- a/icu4c/source/test/intltest/tsmthred.h +++ b/icu4c/source/test/intltest/tsmthred.h @@ -53,6 +53,7 @@ public: void TestUnifiedCache(); void TestBreakTranslit(); void TestIncDec(); + void Test20104(); }; #endif