organicmaps/icu
13
Fork 0
mirror of https://github.com/unicode-org/icu.git synced 2025-04-07 06:25:30 +00:00
Code Issues 1 Projects Releases Wiki Activity Actions 1

ICU-20050 Fixing memory leaks in move and copy assignment in Number*Formatter.

Browse source
This commit is contained in:
Shane Carr 2018-09-18 02:14:24 -07:00
parent f90676c2bc
commit cab92db338
No known key found for this signature in database
GPG key ID: FCED3B24AAB18B5C
7 changed files with 78 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
icu4c/source/i18n/number_fluent.cpp
View file

@ -407,7 +407,8 @@ LocalizedNumberFormatter::LocalizedNumberFormatter(NFS<LNF>&& src) U_NOEXCEPT
LocalizedNumberFormatter& LocalizedNumberFormatter::operator=(const LNF& other) {
NFS<LNF>::operator=(static_cast<const NFS<LNF>&>(other));
// No additional fields to assign (let call count and compiled formatter reset to defaults)
// Reset to default values.
clear();
return *this;
}
@ -419,20 +420,26 @@ LocalizedNumberFormatter& LocalizedNumberFormatter::operator=(LNF&& src) U_NOEXC
// Formatter is compiled
lnfMoveHelper(static_cast<LNF&&>(src));
} else {
// Reset to default values.
auto* callCount = reinterpret_cast<u_atomic_int32_t*>(fUnsafeCallCount);
umtx_storeRelease(*callCount, 0);
fCompiled = nullptr;
clear();
}
return *this;
}
void LocalizedNumberFormatter::clear() {
// Reset to default values.
auto* callCount = reinterpret_cast<u_atomic_int32_t*>(fUnsafeCallCount);
umtx_storeRelease(*callCount, 0);
delete fCompiled;
fCompiled = nullptr;
}
void LocalizedNumberFormatter::lnfMoveHelper(LNF&& src) {
// Copy over the compiled formatter and set call count to INT32_MIN as in computeCompiled().
// Don't copy the call count directly because doing so requires a loadAcquire/storeRelease.
// The bits themselves appear to be platform-dependent, so copying them might not be safe.
auto* callCount = reinterpret_cast<u_atomic_int32_t*>(fUnsafeCallCount);
umtx_storeRelease(*callCount, INT32_MIN);
delete fCompiled;
fCompiled = src.fCompiled;
// Reset the source object to leave it in a safe state.
auto* srcCallCount = reinterpret_cast<u_atomic_int32_t*>(src.fUnsafeCallCount);

14
icu4c/source/i18n/numrange_fluent.cpp
View file

@ -227,18 +227,26 @@ LocalizedNumberRangeFormatter::LocalizedNumberRangeFormatter(LocalizedNumberRang
LocalizedNumberRangeFormatter::LocalizedNumberRangeFormatter(NFS<LNF>&& src) U_NOEXCEPT
: NFS<LNF>(std::move(src)) {
// No additional fields to assign
// Steal the compiled formatter
LNF&& _src = static_cast<LNF&&>(src);
fImpl = _src.fImpl;
_src.fImpl = nullptr;
}
LocalizedNumberRangeFormatter& LocalizedNumberRangeFormatter::operator=(const LNF& other) {
NFS<LNF>::operator=(static_cast<const NFS<LNF>&>(other));
// No additional fields to assign
// Do not steal; just clear
delete fImpl;
fImpl = nullptr;
return *this;
}
LocalizedNumberRangeFormatter& LocalizedNumberRangeFormatter::operator=(LNF&& src) U_NOEXCEPT {
NFS<LNF>::operator=(static_cast<NFS<LNF>&&>(src));
// No additional fields to assign
// Steal the compiled formatter
delete fImpl;
fImpl = src.fImpl;
src.fImpl = nullptr;
return *this;
}

2
icu4c/source/i18n/unicode/numberformatter.h
View file

@ -2358,6 +2358,8 @@ class U_I18N_API LocalizedNumberFormatter
LocalizedNumberFormatter(impl::MacroProps &&macros, const Locale &locale);
void clear();
void lnfMoveHelper(LocalizedNumberFormatter&& src);
/**

2
icu4c/source/i18n/unicode/numberrangeformatter.h
View file

@ -625,6 +625,8 @@ class U_I18N_API LocalizedNumberRangeFormatter
LocalizedNumberRangeFormatter(impl::RangeMacroProps &&macros, const Locale &locale);
void clear();
// To give the fluent setters access to this class's constructor:
friend class NumberRangeFormatterSettings<UnlocalizedNumberRangeFormatter>;
friend class NumberRangeFormatterSettings<LocalizedNumberRangeFormatter>;

1
icu4c/source/test/intltest/numbertest.h
View file

@ -257,6 +257,7 @@ class NumberRangeFormatterTest : public IntlTest {
void testIdentity();
void testDifferentFormatters();
void testPlurals();
void testCopyMove();
void runIndexedTest(int32_t index, UBool exec, const char *&name, char *par = 0);

7
icu4c/source/test/intltest/numbertest_api.cpp
View file

@ -2372,8 +2372,15 @@ void NumberFormatterApiTest::copyMove() {
assertTrue("[constructor] Source should be reset after move", l1.getCompiled() == nullptr);
// Reset l1 and l2 to check for macro-props copying for behavior testing
// Make the test more interesting: also warm them up with a compiled formatter.
l1 = NumberFormatter::withLocale("en");
l1.formatInt(1, status);
l1.formatInt(1, status);
l1.formatInt(1, status);
l2 = NumberFormatter::withLocale("en");
l2.formatInt(1, status);
l2.formatInt(1, status);
l2.formatInt(1, status);
// Copy assignment
l1 = l3;

43
icu4c/source/test/intltest/numbertest_range.cpp
View file

@ -48,6 +48,7 @@ void NumberRangeFormatterTest::runIndexedTest(int32_t index, UBool exec, const c
TESTCASE_AUTO(testIdentity);
TESTCASE_AUTO(testDifferentFormatters);
TESTCASE_AUTO(testPlurals);
TESTCASE_AUTO(testCopyMove);
TESTCASE_AUTO_END;
}
@ -706,6 +707,48 @@ void NumberRangeFormatterTest::testPlurals() {
}
}
void NumberRangeFormatterTest::testCopyMove() {
IcuTestErrorCode status(*this, "testCopyMove");
// Default constructors
LocalizedNumberRangeFormatter l1;
assertEquals("Initial behavior", u"1–5", l1.formatFormattableRange(1, 5, status).toString(status));
if (status.errDataIfFailureAndReset()) { return; }
// Setup
l1 = NumberRangeFormatter::withLocale("fr-FR")
.numberFormatterBoth(NumberFormatter::with().unit(USD));
assertEquals("Currency behavior", u"1,00–5,00 $US", l1.formatFormattableRange(1, 5, status).toString(status));
// Copy constructor
LocalizedNumberRangeFormatter l2 = l1;
assertEquals("Copy constructor", u"1,00–5,00 $US", l2.formatFormattableRange(1, 5, status).toString(status));
// Move constructor
LocalizedNumberRangeFormatter l3 = std::move(l1);
assertEquals("Move constructor", u"1,00–5,00 $US", l3.formatFormattableRange(1, 5, status).toString(status));
// Reset objects for assignment tests
l1 = NumberRangeFormatter::withLocale("en-us");
l2 = NumberRangeFormatter::withLocale("en-us");
assertEquals("Rest behavior, l1", u"1–5", l1.formatFormattableRange(1, 5, status).toString(status));
assertEquals("Rest behavior, l2", u"1–5", l2.formatFormattableRange(1, 5, status).toString(status));
// Copy assignment
l1 = l3;
assertEquals("Copy constructor", u"1,00–5,00 $US", l1.formatFormattableRange(1, 5, status).toString(status));
// Move assignment
l2 = std::move(l3);
assertEquals("Copy constructor", u"1,00–5,00 $US", l2.formatFormattableRange(1, 5, status).toString(status));
// FormattedNumberRange
FormattedNumberRange result = l1.formatFormattableRange(1, 5, status);
assertEquals("FormattedNumberRange move constructor", u"1,00–5,00 $US", result.toString(status));
result = l1.formatFormattableRange(3, 6, status);
assertEquals("FormattedNumberRange move assignment", u"3,00–6,00 $US", result.toString(status));
}
void NumberRangeFormatterTest::assertFormatRange(
const char16_t* message,
const UnlocalizedNumberRangeFormatter& f,