Review Request: CR 7100996 - (spec str) IndexOutOfBoundsException when using a StringBuffer from multiple threads

[Ulf Zibis]

Oops crossposting.

Looks much better. Maybe you still consider to compare with my suggestions.
IMO you do not need to use the "(source)" parts, consistent to the paragraphs above.

-Ulf


Am 21.06.2012 22:44, schrieb Jim Gish:
> I did this, and also simplified the language a bit:
>
> diff -r 7722e2bec02b src/share/classes/java/lang/StringBuffer.java
> --- a/src/share/classes/java/lang/StringBuffer.java    Thu Jun 21 16:26:04 2012 -0400
> +++ b/src/share/classes/java/lang/StringBuffer.java    Thu Jun 21 16:42:19 2012 -0400
> @@ -63,14 +63,15 @@
>   * appending or inserting from a source sequence) this class synchronizes
>   * only on the string buffer performing the operation, not on the source.
>   * <p>
> + * Although {@code StringBuffer} is designed to be safe to use
> + * concurrently from multiple threads, if the source data passed
> + * to the constructor, i.e. {@code StringBuffer(source)}, or to the
> + * {@code append(source)}, or {@code insert(source)} operations
> + * is shared across threads, it must be ensured that the operations have
> + * a consistent and unchanging view of the source data for the duration
> + * of the operation.
> + * This could be satisfied by the caller holding a lock during the
> + * operation's call, or because the source data is
>   * immutable, or because the source data is not shared across threads.
>   * <p>
>   * Every string buffer has a capacity. As long as the length of the
>
>
>
> Thanks,
>    Jim
>
> On 06/21/2012 02:55 PM, Mike Duigou wrote:
>> Great addition!
>>
>> I believe you should be using {@code} rather than<code> tags.
>>
>> I would say "constructors" rather than "create".
>>
>> I would add the word "operation" after the first instance of "create/append/insert"
>>
>> I would change "This could be done" to "This could be satisfied"
>>
>> Mike
>>
>> On Jun 21 2012, at 11:10 , Jim Gish wrote:
>>
>>> Taking all the previous comments into consideration, here's an update:
>>>
>>> diff -r fc575c78f5d3 src/share/classes/java/lang/StringBuffer.java
>>> --- a/src/share/classes/java/lang/StringBuffer.java    Sun Jun 10 10:29:27 2012 +0100
>>> +++ b/src/share/classes/java/lang/StringBuffer.java    Thu Jun 21 14:09:17 2012 -0400
>>> @@ -63,6 +63,16 @@
>>>   * appending or inserting from a source sequence) this class synchronizes
>>>   * only on the string buffer performing the operation, not on the source.
>>>   *<p>
>>> + * While<code>StringBuffer</code>  is designed to be safe to use
>>> + * concurrently from multiple threads, the source data passed to create,
>>> + * i.e.<code>StringBuffer(source)</code>,<code>append(source)</code>,
>>> + * or<code>insert(source)</code>, if shared across threads, must ensure
>>> + * that<code>create/append/insert</code>  has a consistent and unchanging
>>> + * view of the source data for the duration of the operation.  This could
>>> + * be done by the caller holding a lock during the
>>> + *<code>create/append/insert</code>  call, or because the source data is
>>> + * immutable, or because the source data is not shared across threads.
>>> + *<p>
>>>   * Every string buffer has a capacity. As long as the length of the
>>>   * character sequence contained in the string buffer does not exceed
>>>   * the capacity, it is not necessary to allocate a new internal
>>>
>>> Thanks,
>>>    Jim
>>>
>>> On 06/21/2012 12:49 PM, David Schlosnagle wrote:
>>>> On Thu, Jun 21, 2012 at 11:59 AM, Jim Gish<jim.gish at oracle.com>   wrote:
>>>>> + * across threads, must ensure that StringBuffer.add/insert has a
>>>> Jim,
>>>>
>>>> You may want to replace "add" with "append" if you are intending to
>>>> reference the actual method name and not the generic operation.
>>>> Additionally, you may want to use {@code ...} to show this context.
>>>>
>>>> Thanks,
>>>> Dave
>>> -- 
>>> Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304
>>> Oracle Java Platform Group | Core Libraries Team
>>> 35 Network Drive
>>> Burlington, MA 01803
>>> jim.gish at oracle.com
>>>
>