RFR: 8354522: Clones of DecimalFormat cause interferences when used concurrently [v2]

[Justin Lu]

On Tue, 15 Apr 2025 14:43:56 GMT, Johannes Graham <duke at openjdk.org> wrote:

>> The `DigitList` class used in `DecimalFormat` does not reset the `data` array in its clone method. This can cause interference when clones are used concurrently.
>
> Johannes Graham has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - fix test summary
>  - add test

Thanks for the fix, it looks good. Left some minor comments. I will run tiers 1-3 with your change.

src/java.base/share/classes/java/text/DigitList.java line 728:

> 726:             System.arraycopy(digits, 0, newDigits, 0, digits.length);
> 727:             other.digits = newDigits;
> 728:             other.data = null;

We don't have to copy the data _array_ unlike the _digits_ array (above) because it can just be reset during the next `getDataChars` call, but a comment as to why might be helpful.

test/jdk/java/text/Format/DecimalFormat/CloneTest.java line 27:

> 25:  * @test
> 26:  * @bug 8354522
> 27:  * @summary Check for cloning interference

It will probably be good to mention somewhere that this test/fix addresses the issue of the same _data_ array reference being shared amongst DigitList clones.

test/jdk/java/text/Format/DecimalFormat/CloneTest.java line 48:

> 46:         AtomicInteger mismatchCount = new AtomicInteger(0);
> 47:         DecimalFormat df = new DecimalFormat("#");
> 48:         String _ = df.format(Math.PI); // initial use of formatter

We should probably comment the importance of this line, as without it the test will pass without the fix. (It sets the _data_ array to a non null value).

test/jdk/java/text/Format/DecimalFormat/CloneTest.java line 54:

> 52:                 DecimalFormat threadDf = (DecimalFormat) df.clone();
> 53:                 Runnable task = () -> {
> 54:                     for (int j = 0; j < 1000000; j++) {

We should probably make this test less costly by decreasing some values, I don't the bug requires that many iterations to be exposed. (Without the fix and the `break` statement in the test, `mismatchCount` gets up into the tens of thousands on my machine.)

-------------

PR Review: https://git.openjdk.org/jdk/pull/24598#pullrequestreview-2769119077
PR Review Comment: https://git.openjdk.org/jdk/pull/24598#discussion_r2045055262
PR Review Comment: https://git.openjdk.org/jdk/pull/24598#discussion_r2045062899
PR Review Comment: https://git.openjdk.org/jdk/pull/24598#discussion_r2045083581
PR Review Comment: https://git.openjdk.org/jdk/pull/24598#discussion_r2045060617