organicmaps/icu
13
Fork 0
mirror of https://github.com/unicode-org/icu.git synced 2025-04-13 17:01:16 +00:00
Code Issues 1 Projects Releases Wiki Activity Actions 2

ICU-11669 DateIntervalFormat::format() thread safety fixes.

Browse source 
X-SVN-Rev: 38151
This commit is contained in:
Andy Heninger 2016-01-06 00:09:25 +00:00
parent ec1aaee041
commit 3e1bf369df
4 changed files with 206 additions and 107 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

192
icu4c/source/i18n/dtitvfmt.cpp
View file

@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (C) 2008-2015, International Business Machines Corporation and
* Copyright (C) 2008-2016, International Business Machines Corporation and
* others. All Rights Reserved.
*******************************************************************************
*
@ -17,20 +17,20 @@
//TODO: put in compilation
//#define DTITVFMT_DEBUG 1
#include "cstring.h"
#include "unicode/msgfmt.h"
#include "unicode/dtptngen.h"
#include "unicode/dtitvinf.h"
#include "unicode/calendar.h"
#include "cstring.h"
#include "dtitv_impl.h"
#include "gregoimp.h"
#include "mutex.h"
#ifdef DTITVFMT_DEBUG
#include <iostream>
#include "cstring.h"
#endif
#include "gregoimp.h"
U_NAMESPACE_BEGIN
@ -63,7 +63,10 @@ static const UChar gEarlierFirstPrefix[] = {LOW_E, LOW_A, LOW_R, LOW_L, LOW_I, L
UOBJECT_DEFINE_RTTI_IMPLEMENTATION(DateIntervalFormat)
// Mutex, protects access to fDateFormat, fFromCalendar and fToCalendar.
// Needed because these data members are modified by const methods of DateIntervalFormat.
static UMutex gFormatterMutex = U_MUTEX_INITIALIZER;
DateIntervalFormat* U_EXPORT2
DateIntervalFormat::createInstance(const UnicodeString& skeleton,
@ -148,26 +151,29 @@ DateIntervalFormat::operator=(const DateIntervalFormat& itvfmt) {
delete fDatePattern;
delete fTimePattern;
delete fDateTimeFormat;
if ( itvfmt.fDateFormat ) {
fDateFormat = (SimpleDateFormat*)itvfmt.fDateFormat->clone();
} else {
fDateFormat = NULL;
{
Mutex lock(&gFormatterMutex);
if ( itvfmt.fDateFormat ) {
fDateFormat = (SimpleDateFormat*)itvfmt.fDateFormat->clone();
} else {
fDateFormat = NULL;
}
if ( itvfmt.fFromCalendar ) {
fFromCalendar = itvfmt.fFromCalendar->clone();
} else {
fFromCalendar = NULL;
}
if ( itvfmt.fToCalendar ) {
fToCalendar = itvfmt.fToCalendar->clone();
} else {
fToCalendar = NULL;
}
}
if ( itvfmt.fInfo ) {
fInfo = itvfmt.fInfo->clone();
} else {
fInfo = NULL;
}
if ( itvfmt.fFromCalendar ) {
fFromCalendar = itvfmt.fFromCalendar->clone();
} else {
fFromCalendar = NULL;
}
if ( itvfmt.fToCalendar ) {
fToCalendar = itvfmt.fToCalendar->clone();
} else {
fToCalendar = NULL;
}
fSkeleton = itvfmt.fSkeleton;
int8_t i;
for ( i = 0; i< DateIntervalInfo::kIPI_MAX_INDEX; ++i ) {
@ -201,50 +207,41 @@ DateIntervalFormat::clone(void) const {
UBool
DateIntervalFormat::operator==(const Format& other) const {
if (typeid(*this) == typeid(other)) {
const DateIntervalFormat* fmt = (DateIntervalFormat*)&other;
#ifdef DTITVFMT_DEBUG
UBool equal;
equal = (this == fmt);
equal = (*fInfo == *fmt->fInfo);
equal = (*fDateFormat == *fmt->fDateFormat);
equal = fFromCalendar->isEquivalentTo(*fmt->fFromCalendar) ;
equal = fToCalendar->isEquivalentTo(*fmt->fToCalendar) ;
equal = (fSkeleton == fmt->fSkeleton);
equal = ((fDatePattern == NULL && fmt->fDatePattern == NULL) || (fDatePattern && fmt->fDatePattern && *fDatePattern == *fmt->fDatePattern));
equal = ((fTimePattern == NULL && fmt->fTimePattern == NULL) || (fTimePattern && fmt->fTimePattern && *fTimePattern == *fmt->fTimePattern));
equal = ((fDateTimeFormat == NULL && fmt->fDateTimeFormat == NULL) || (fDateTimeFormat && fmt->fDateTimeFormat && *fDateTimeFormat == *fmt->fDateTimeFormat));
#endif
UBool res;
res = ( this == fmt ) ||
( Format::operator==(other) &&
fInfo &&
( *fInfo == *fmt->fInfo ) &&
fDateFormat &&
( *fDateFormat == *fmt->fDateFormat ) &&
fFromCalendar &&
fFromCalendar->isEquivalentTo(*fmt->fFromCalendar) &&
fToCalendar &&
fToCalendar->isEquivalentTo(*fmt->fToCalendar) &&
fSkeleton == fmt->fSkeleton &&
((fDatePattern == NULL && fmt->fDatePattern == NULL) || (fDatePattern && fmt->fDatePattern && *fDatePattern == *fmt->fDatePattern)) &&
((fTimePattern == NULL && fmt->fTimePattern == NULL) || (fTimePattern && fmt->fTimePattern && *fTimePattern == *fmt->fTimePattern)) &&
((fDateTimeFormat == NULL && fmt->fDateTimeFormat == NULL) || (fDateTimeFormat && fmt->fDateTimeFormat && *fDateTimeFormat == *fmt->fDateTimeFormat)) && fLocale == fmt->fLocale);
int8_t i;
for (i = 0; i< DateIntervalInfo::kIPI_MAX_INDEX && res == TRUE; ++i ) {
res = ( fIntervalPatterns[i].firstPart ==
fmt->fIntervalPatterns[i].firstPart) &&
( fIntervalPatterns[i].secondPart ==
fmt->fIntervalPatterns[i].secondPart ) &&
( fIntervalPatterns[i].laterDateFirst ==
fmt->fIntervalPatterns[i].laterDateFirst) ;
}
return res;
}
return FALSE;
}
if (typeid(*this) != typeid(other)) {return FALSE;}
const DateIntervalFormat* fmt = (DateIntervalFormat*)&other;
if (this == fmt) {return TRUE;}
if (!Format::operator==(other)) {return FALSE;}
if ((fInfo != fmt->fInfo) && (fInfo == NULL || fmt->fInfo == NULL)) {return FALSE;}
if (fInfo && fmt->fInfo && (*fInfo != *fmt->fInfo )) {return FALSE;}
{
Mutex lock(&gFormatterMutex);
if (fDateFormat != fmt->fDateFormat && (fDateFormat == NULL || fmt->fDateFormat == NULL)) {return FALSE;}
if (fDateFormat && fmt->fDateFormat && (*fDateFormat != *fmt->fDateFormat)) {return FALSE;}
// TODO: should operator == ignore the From and ToCalendar? They hold transient values during
// formatting of a DateInterval.
if (fFromCalendar != fmt->fFromCalendar && (fFromCalendar == NULL || fmt->fFromCalendar == NULL)) {return FALSE;}
if (fFromCalendar && fmt->fFromCalendar && !fFromCalendar->isEquivalentTo(*fmt->fFromCalendar)) {return FALSE;}
if (fToCalendar != fmt->fToCalendar && (fToCalendar == NULL || fmt->fToCalendar == NULL)) {return FALSE;}
if (fToCalendar && fmt->fToCalendar && !fToCalendar->isEquivalentTo(*fmt->fToCalendar)) {return FALSE;}
}
if (fSkeleton != fmt->fSkeleton) {return FALSE;}
if (fDatePattern != fmt->fDatePattern && (fDatePattern == NULL || fmt->fDatePattern == NULL)) {return FALSE;}
if (fDatePattern && fmt->fDatePattern && (*fDatePattern != *fmt->fDatePattern)) {return FALSE;}
if (fTimePattern != fmt->fTimePattern && (fTimePattern == NULL || fmt->fTimePattern == NULL)) {return FALSE;}
if (fTimePattern && fmt->fTimePattern && (*fTimePattern != *fmt->fTimePattern)) {return FALSE;}
if (fDateTimeFormat != fmt->fDateTimeFormat && (fDateTimeFormat == NULL || fmt->fDateTimeFormat == NULL)) {return FALSE;}
if (fDateTimeFormat && fmt->fDateTimeFormat && (*fDateTimeFormat != *fmt->fDateTimeFormat)) {return FALSE;}
if (fLocale != fmt->fLocale) {return FALSE;}
for (int32_t i = 0; i< DateIntervalInfo::kIPI_MAX_INDEX; ++i ) {
if (fIntervalPatterns[i].firstPart != fmt->fIntervalPatterns[i].firstPart) {return FALSE;}
if (fIntervalPatterns[i].secondPart != fmt->fIntervalPatterns[i].secondPart ) {return FALSE;}
if (fIntervalPatterns[i].laterDateFirst != fmt->fIntervalPatterns[i].laterDateFirst) {return FALSE;}
}
return TRUE;
}
UnicodeString&
@ -259,7 +256,7 @@ DateIntervalFormat::format(const Formattable& obj,
if ( obj.getType() == Formattable::kObject ) {
const UObject* formatObj = obj.getObject();
const DateInterval* interval = dynamic_cast<const DateInterval*>(formatObj);
if (interval != NULL){
if (interval != NULL) {
return format(interval, appendTo, fieldPosition, status);
}
}
@ -276,16 +273,15 @@ DateIntervalFormat::format(const DateInterval* dtInterval,
if ( U_FAILURE(status) ) {
return appendTo;
}
if ( fFromCalendar != NULL && fToCalendar != NULL &&
fDateFormat != NULL && fInfo != NULL ) {
fFromCalendar->setTime(dtInterval->getFromDate(), status);
fToCalendar->setTime(dtInterval->getToDate(), status);
if ( U_SUCCESS(status) ) {
return format(*fFromCalendar, *fToCalendar, appendTo,fieldPosition, status);
}
if (fFromCalendar == NULL || fToCalendar == NULL || fDateFormat == NULL || fInfo == NULL) {
status = U_INVALID_STATE_ERROR;
return appendTo;
}
return appendTo;
Mutex lock(&gFormatterMutex);
fFromCalendar->setTime(dtInterval->getFromDate(), status);
fToCalendar->setTime(dtInterval->getToDate(), status);
return formatImpl(*fFromCalendar, *fToCalendar, appendTo,fieldPosition, status);
}
@ -295,6 +291,17 @@ DateIntervalFormat::format(Calendar& fromCalendar,
UnicodeString& appendTo,
FieldPosition& pos,
UErrorCode& status) const {
Mutex lock(&gFormatterMutex);
return formatImpl(fromCalendar, toCalendar, appendTo, pos, status);
}
UnicodeString&
DateIntervalFormat::formatImpl(Calendar& fromCalendar,
Calendar& toCalendar,
UnicodeString& appendTo,
FieldPosition& pos,
UErrorCode& status) const {
if ( U_FAILURE(status) ) {
return appendTo;
}
@ -436,7 +443,7 @@ DateIntervalFormat::setDateIntervalInfo(const DateIntervalInfo& newItvPattern,
delete fDateTimeFormat;
fDateTimeFormat = NULL;
if ( fDateFormat ) {
if (fDateFormat) {
initializePattern(status);
}
}
@ -487,6 +494,7 @@ const TimeZone&
DateIntervalFormat::getTimeZone() const
{
if (fDateFormat != NULL) {
Mutex lock(&gFormatterMutex);
return fDateFormat->getTimeZone();
}
// If fDateFormat is NULL (unexpected), create default timezone.
@ -506,37 +514,21 @@ DateIntervalFormat::DateIntervalFormat(const Locale& locale,
fTimePattern(NULL),
fDateTimeFormat(NULL)
{
if ( U_FAILURE(status) ) {
delete dtItvInfo;
return;
}
SimpleDateFormat* dtfmt =
static_cast<SimpleDateFormat *>(
DateFormat::createInstanceForSkeleton(
*skeleton, locale, status));
if ( U_FAILURE(status) ) {
delete dtItvInfo;
delete dtfmt;
return;
}
if ( dtfmt == NULL || dtItvInfo == NULL) {
status = U_MEMORY_ALLOCATION_ERROR;
// safe to delete NULL
delete dtfmt;
delete dtItvInfo;
LocalPointer<DateIntervalInfo> info(dtItvInfo, status);
LocalPointer<SimpleDateFormat> dtfmt(static_cast<SimpleDateFormat *>(
DateFormat::createInstanceForSkeleton(*skeleton, locale, status)), status);
if (U_FAILURE(status)) {
return;
}
if ( skeleton ) {
fSkeleton = *skeleton;
}
fInfo = dtItvInfo;
fDateFormat = dtfmt;
if ( dtfmt->getCalendar() ) {
fFromCalendar = dtfmt->getCalendar()->clone();
fToCalendar = dtfmt->getCalendar()->clone();
} else {
fFromCalendar = NULL;
fToCalendar = NULL;
fInfo = info.orphan();
fDateFormat = dtfmt.orphan();
if ( fDateFormat->getCalendar() ) {
fFromCalendar = fDateFormat->getCalendar()->clone();
fToCalendar = fDateFormat->getCalendar()->clone();
}
initializePattern(status);
}
@ -771,7 +763,7 @@ DateIntervalFormat::initializePattern(UErrorCode& status) {
* range expression for the time.
*/
if ( fDateTimeFormat == 0 ) {
if ( fDateTimeFormat == NULL ) {
// earlier failure getting dateTimeFormat
return;
}

44
icu4c/source/i18n/unicode/dtitvfmt.h
View file

@ -1,5 +1,5 @@
/********************************************************************************
* Copyright (C) 2008-2013,2015, International Business Machines Corporation and
* Copyright (C) 2008-2016, International Business Machines Corporation and
* others. All Rights Reserved.
*******************************************************************************
*
@ -504,7 +504,13 @@ public:
/**
* Gets the date formatter
* Gets the date formatter. The DateIntervalFormat instance continues to own
* the returned DateFormatter object, and will use and possibly modify it
* during format operations. In a multi-threaded environment, the returned
* DateFormat can only be used if it is certain that no other threads are
* concurrently using this DateIntervalFormatter, even for nominally const
* functions.
*
* @return the date formatter associated with this date interval formatter.
* @stable ICU 4.0
*/
@ -685,6 +691,8 @@ private:
* The full pattern used in this fall-back format is the
* full pattern of the date formatter.
*
* gFormatterMutex must already be locked when calling this function.
*
* @param fromCalendar calendar set to the from date in date interval
* to be formatted into date interval string
* @param toCalendar calendar set to the to date in date interval
@ -697,6 +705,7 @@ private:
* On output: the offsets of the alignment field.
* @param status output param set to success/failure code on exit
* @return Reference to 'appendTo' parameter.
* @internal
*/
UnicodeString& fallbackFormat(Calendar& fromCalendar,
Calendar& toCalendar,
@ -952,6 +961,37 @@ private:
const UnicodeString* secondPart,
UBool laterDateFirst);
/**
* Format 2 Calendars to produce a string.
* Implementation of the similar public format function.
* Must be called with gFormatterMutex already locked.
*
* Note: "fromCalendar" and "toCalendar" are not const,
* since calendar is not const in SimpleDateFormat::format(Calendar&),
*
* @param fromCalendar calendar set to the from date in date interval
* to be formatted into date interval string
* @param toCalendar calendar set to the to date in date interval
* to be formatted into date interval string
* @param appendTo Output parameter to receive result.
* Result is appended to existing contents.
* @param fieldPosition On input: an alignment field, if desired.
* On output: the offsets of the alignment field.
* There may be multiple instances of a given field type
* in an interval format; in this case the fieldPosition
* offsets refer to the first instance.
* @param status Output param filled with success/failure status.
* Caller needs to make sure it is SUCCESS
* at the function entrance
* @return Reference to 'appendTo' parameter.
* @internal
*/
UnicodeString& formatImpl(Calendar& fromCalendar,
Calendar& toCalendar,
UnicodeString& appendTo,
FieldPosition& fieldPosition,
UErrorCode& status) const ;
// from calendar field to pattern letter
static const UChar fgCalendarFieldToPatternLetter[];

72
icu4c/source/test/intltest/dtifmtts.cpp
View file

@ -1,7 +1,7 @@
/********************************************************************
* COPYRIGHT:
* Copyright (c) 1997-2015, International Business Machines Corporation and
* Copyright (c) 1997-2016, International Business Machines Corporation and
* others. All Rights Reserved.
********************************************************************/
@ -18,9 +18,11 @@
#include <iostream>
#endif
#include "cstring.h"
#include "dtifmtts.h"
#include "cstr.h"
#include "cstring.h"
#include "simplethread.h"
#include "unicode/gregocal.h"
#include "unicode/dtintrv.h"
#include "unicode/dtitvinf.h"
@ -51,6 +53,7 @@ void DateIntervalFormatTest::runIndexedTest( int32_t index, UBool exec, const ch
TESTCASE(5, testStress);
TESTCASE(6, testTicket11583_2);
TESTCASE(7, testTicket11985);
TESTCASE(8, testTicket11669);
default: name = ""; break;
}
}
@ -128,7 +131,7 @@ void DateIntervalFormatTest::testAPI() {
DateIntervalFormat* another = (DateIntervalFormat*)dtitvfmt->clone();
if ( (*another) != (*dtitvfmt) ) {
dataerrln("ERROR: clone failed");
dataerrln("%s:%d ERROR: clone failed", __FILE__, __LINE__);
}
@ -1544,4 +1547,65 @@ void DateIntervalFormatTest::testTicket11985() {
assertEquals("Format pattern", "h:mm a", pattern);
}
// Ticket 11669 - thread safety of DateIntervalFormat::format(). This test failed before
// the implementation was fixed.
static const DateIntervalFormat *gIntervalFormatter = NULL; // The Formatter to be used concurrently by test threads.
static const DateInterval *gInterval = NULL; // The date interval to be formatted concurrently.
static const UnicodeString *gExpectedResult = NULL;
void DateIntervalFormatTest::threadFunc11669(int32_t threadNum) {
(void)threadNum;
for (int loop=0; loop<1000; ++loop) {
UErrorCode status = U_ZERO_ERROR;
FieldPosition pos(0);
UnicodeString result;
gIntervalFormatter->format(gInterval, result, pos, status);
if (U_FAILURE(status)) {
errln("%s:%d %s", __FILE__, __LINE__, u_errorName(status));
return;
}
if (result != *gExpectedResult) {
errln("%s:%d Expected \"%s\", got \"%s\"", __FILE__, __LINE__, CStr(*gExpectedResult)(), CStr(result)());
return;
}
}
}
void DateIntervalFormatTest::testTicket11669() {
UErrorCode status = U_ZERO_ERROR;
LocalPointer<DateIntervalFormat> formatter(DateIntervalFormat::createInstance(UDAT_YEAR_MONTH_DAY, Locale::getEnglish(), status), status);
LocalPointer<TimeZone> tz(TimeZone::createTimeZone("America/Los_Angleles"), status);
LocalPointer<Calendar> intervalStart(Calendar::createInstance(*tz, Locale::getEnglish(), status), status);
LocalPointer<Calendar> intervalEnd(Calendar::createInstance(*tz, Locale::getEnglish(), status), status);
if (U_FAILURE(status)) {
errln("%s:%d %s", __FILE__, __LINE__, u_errorName(status));
return;
}
intervalStart->set(2009, 6, 1, 14, 0);
intervalEnd->set(2009, 6, 2, 14, 0);
DateInterval interval(intervalStart->getTime(status), intervalEnd->getTime(status));
FieldPosition pos(0);
UnicodeString expectedResult;
formatter->format(&interval, expectedResult, pos, status);
if (U_FAILURE(status)) {
errln("%s:%d %s", __FILE__, __LINE__, u_errorName(status));
return;
}
gInterval = &interval;
gIntervalFormatter = formatter.getAlias();
gExpectedResult = &expectedResult;
ThreadPool<DateIntervalFormatTest> threads(this, 4, &DateIntervalFormatTest::threadFunc11669);
threads.start();
threads.join();
gInterval = NULL; // Don't leave dangling pointers lying around. Not strictly necessary.
gIntervalFormatter = NULL;
gExpectedResult = NULL;
}
#endif /* #if !UCONFIG_NO_FORMATTING */

5
icu4c/source/test/intltest/dtifmtts.h
View file

@ -1,6 +1,6 @@
/********************************************************************
* COPYRIGHT:
* Copyright (c) 2008-2015 International Business Machines Corporation and
* Copyright (c) 2008-2016 International Business Machines Corporation and
* others. All Rights Reserved.
********************************************************************/
@ -56,6 +56,9 @@ public:
void testTicket11985();
void testTicket11669();
void threadFunc11669(int32_t threadNum);
private:
/**
* Test formatting against expected result