On Tue, 15 Apr 2025 16:49:57 GMT, Justin Lu <j...@openjdk.org> wrote:
>> Johannes Graham has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - fix test summary
>> - add test
>
> 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.
I added more detail to the comment with the test method.
> 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).
Done
> 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.)
For the original reproducer I wanted to be _really_ sure that it failed. I've
reduced it to be more reasonable. With the current config it still gets 10-100
mismatches for me. I've limited the number of lines that get logged as well.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24598#discussion_r2045232675
PR Review Comment: https://git.openjdk.org/jdk/pull/24598#discussion_r2045232933
PR Review Comment: https://git.openjdk.org/jdk/pull/24598#discussion_r2045235681
