RFR: 8349241: Fix the concurrent execution JVM crash of StringBuilder::append(int/long) [v5]

[Roger Riggs]

On Tue, 4 Feb 2025 14:03:24 GMT, Shaojin Wen <swen at openjdk.org> wrote:

>> The following code can reproduce the problem, writing out of bounds causes JVM Crash
>> 
>> 
>>          StringBuilder buf = new StringBuilder();
>>         buf.append('中');
>> 
>>         Thread[] threads = new Thread[40];
>>         final CountDownLatch latch = new CountDownLatch(threads.length);
>>         Runnable r = () -> {
>>             for (int i = 0; i < 1000000; i++) {
>>                 buf.setLength(0);
>>                 buf.trimToSize();
>>                 buf.append(123456789123456789L);
>>             }
>>             latch.countDown();
>>         };
>> 
>>         for (int i = 0; i < threads.length; i++) {
>>             threads[i] = new Thread(r);
>>         }
>>         for (Thread t : threads) {
>>             t.start();
>>         }
>>         latch.await();
>> 
>> 
>> This problem can be avoided by using the value of ensureCapacityInternal directly.
>
> Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   improved coder thread safe

src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 253:

> 251:      * If {@code minimumCapacity} is non positive due to numeric
> 252:      * overflow, this method throws {@code OutOfMemoryError}.
> 253:      */

Complete the javadoc with the @param tags and descriptions.

src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 256:

> 254:     private byte[] ensureCapacityInternal(int minimumCapacity, byte coder) {
> 255:         // overflow-conscious code
> 256:         byte[] value = this.value;

Shadowing `value` is a bug prone, pick a new name.

src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 278:

> 276:      *         greater than (Integer.MAX_VALUE >> coder)
> 277:      */
> 278:     private static int newCapacity(int minCapacity, byte[] value, byte coder) {

1.  Only the current length is needed and should be the argument.
2. Add the @param tags for the new arguments.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23427#discussion_r1941675018
PR Review Comment: https://git.openjdk.org/jdk/pull/23427#discussion_r1941673018
PR Review Comment: https://git.openjdk.org/jdk/pull/23427#discussion_r1941669127