Skip to content

Commit bdf6853

Browse files
committed
8368328: CompactNumberFormat.clone does not produce independent instances
Reviewed-by: rgiulietti, jlu
1 parent aa6ff45 commit bdf6853

File tree

2 files changed

+114
-8
lines changed

2 files changed

+114
-8
lines changed

src/java.base/share/classes/java/text/CompactNumberFormat.java

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -250,38 +250,43 @@ public final class CompactNumberFormat extends NumberFormat {
250250

251251
/**
252252
* List of positive prefix patterns of this formatter's
253-
* compact number patterns.
253+
* compact number patterns. This field is a read-only
254+
* constant once initialized.
254255
*/
255256
private transient List<Patterns> positivePrefixPatterns;
256257

257258
/**
258259
* List of negative prefix patterns of this formatter's
259-
* compact number patterns.
260+
* compact number patterns. This field is a read-only
261+
* constant once initialized.
260262
*/
261263
private transient List<Patterns> negativePrefixPatterns;
262264

263265
/**
264266
* List of positive suffix patterns of this formatter's
265-
* compact number patterns.
267+
* compact number patterns. This field is a read-only
268+
* constant once initialized.
266269
*/
267270
private transient List<Patterns> positiveSuffixPatterns;
268271

269272
/**
270273
* List of negative suffix patterns of this formatter's
271-
* compact number patterns.
274+
* compact number patterns. This field is a read-only
275+
* constant once initialized.
272276
*/
273277
private transient List<Patterns> negativeSuffixPatterns;
274278

275279
/**
276280
* List of divisors of this formatter's compact number patterns.
277281
* Divisor can be either Long or BigInteger (if the divisor value goes
278-
* beyond long boundary)
282+
* beyond long boundary). This field is a read-only constant
283+
* once initialized.
279284
*/
280285
private transient List<Number> divisors;
281286

282287
/**
283288
* List of place holders that represent minimum integer digits at each index
284-
* for each count.
289+
* for each count. This field is a read-only constant once initialized.
285290
*/
286291
private transient List<Patterns> placeHolderPatterns;
287292

@@ -374,7 +379,7 @@ public final class CompactNumberFormat extends NumberFormat {
374379

375380
/**
376381
* The map for plural rules that maps LDML defined tags (e.g. "one") to
377-
* its rule.
382+
* its rule. This field is a read-only constant once initialized.
378383
*/
379384
private transient Map<String, String> rulesMap;
380385

@@ -1515,7 +1520,7 @@ private void applyPattern(String count, String pattern, int index) {
15151520
}
15161521
}
15171522

1518-
private final transient DigitList digitList = new DigitList();
1523+
private transient DigitList digitList = new DigitList();
15191524
private static final int STATUS_INFINITE = 0;
15201525
private static final int STATUS_POSITIVE = 1;
15211526
private static final int STATUS_LENGTH = 2;
@@ -2506,8 +2511,14 @@ public String toString() {
25062511
@Override
25072512
public CompactNumberFormat clone() {
25082513
CompactNumberFormat other = (CompactNumberFormat) super.clone();
2514+
2515+
// Cloning reference fields. Other fields (e.g., "positivePrefixPatterns")
2516+
// are not cloned since they are read-only constants after initialization.
25092517
other.compactPatterns = compactPatterns.clone();
25102518
other.symbols = (DecimalFormatSymbols) symbols.clone();
2519+
other.decimalFormat = (DecimalFormat) decimalFormat.clone();
2520+
other.defaultDecimalFormat = (DecimalFormat) defaultDecimalFormat.clone();
2521+
other.digitList = (DigitList) digitList.clone();
25112522
return other;
25122523
}
25132524

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/*
2+
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
/*
24+
* @test
25+
* @bug 8368328
26+
* @summary Tests if CompactNumberFormat.clone() creates an independent object
27+
* @run junit/othervm --add-opens java.base/java.text=ALL-UNNAMED TestClone
28+
*/
29+
30+
import java.lang.invoke.MethodHandles;
31+
import java.text.CompactNumberFormat;
32+
import java.text.DecimalFormat;
33+
import java.text.DecimalFormatSymbols;
34+
import java.text.NumberFormat;
35+
import java.util.Locale;
36+
import java.util.stream.IntStream;
37+
import java.util.stream.Stream;
38+
39+
import org.junit.jupiter.api.Test;
40+
import org.junit.jupiter.params.ParameterizedTest;
41+
import org.junit.jupiter.params.provider.Arguments;
42+
import org.junit.jupiter.params.provider.MethodSource;
43+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
44+
import static org.junit.jupiter.api.Assertions.assertEquals;
45+
import static org.junit.jupiter.api.Assertions.assertNotSame;
46+
47+
public class TestClone {
48+
// Concurrently parse numbers using cloned instances as originally
49+
// reported in the bug. This test could produce false negative results,
50+
// depending on the testing environment
51+
@Test
52+
void randomAccessTest() {
53+
var original =
54+
NumberFormat.getCompactNumberInstance(Locale.US, NumberFormat.Style.SHORT);
55+
var threads = IntStream.range(0, 10)
56+
.mapToObj(num -> new Thread(() -> {
57+
var clone = (NumberFormat) original.clone();
58+
for (int i = 0; i < 1000; i++) {
59+
assertDoesNotThrow(() ->
60+
assertEquals(num, clone.parse(String.valueOf(num)).intValue()));
61+
}
62+
})).toList();
63+
threads.forEach(Thread::start);
64+
threads.forEach(t -> {
65+
try {
66+
t.join();
67+
} catch (InterruptedException ie) {
68+
throw new RuntimeException(ie);
69+
}
70+
});
71+
}
72+
73+
private static Stream<Arguments> referenceFields() throws ClassNotFoundException {
74+
return Stream.of(
75+
Arguments.of("compactPatterns", String[].class),
76+
Arguments.of("symbols", DecimalFormatSymbols.class),
77+
Arguments.of("decimalFormat", DecimalFormat.class),
78+
Arguments.of("defaultDecimalFormat", DecimalFormat.class),
79+
Arguments.of("digitList", Class.forName("java.text.DigitList"))
80+
);
81+
}
82+
// Explicitly checks if the cloned object has its own references for
83+
// "compactPatterns", "symbols", "decimalFormat", "defaultDecimalFormat",
84+
// and "digitList"
85+
@ParameterizedTest
86+
@MethodSource("referenceFields")
87+
void whiteBoxTest(String fieldName, Class<?> type) throws Throwable {
88+
var original = NumberFormat.getCompactNumberInstance(Locale.US, NumberFormat.Style.SHORT);
89+
var clone = original.clone();
90+
var lookup = MethodHandles.privateLookupIn(CompactNumberFormat.class, MethodHandles.lookup());
91+
92+
assertNotSame(lookup.findGetter(CompactNumberFormat.class, fieldName, type).invoke(original),
93+
lookup.findGetter(CompactNumberFormat.class, fieldName, type).invoke(clone));
94+
}
95+
}

0 commit comments

Comments
 (0)