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

Mon Jun 25 21:34:19 UTC 2012

Hi Jim,

maybe you like to read some more comments...

I'm not sure, wich would be better, but IMO should be used consistent:
    {@code append()}
    {@code append}

I think, the double quotes belong to the code for "{@code start}" etc. --> {@code "start"}

I do not understand, why you change from term "source sequence" to term "source data" at some point.

Problem with commas, grammar and wording:
  * concurrently from multiple threads, but if the source data passed
  * to the constructor  or to the {@code append()}, or {@code insert()}
  * operations is shared across threads, the calling code must be ensure
  * 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
I would write:
  * concurrently from multiple threads, but if the source data, passed
  * to a constructor, to the {@code append()} or {@code insert()}
  * operations, is shared across threads, the calling code must ensure,
  * that they have a consistent and unchanging view on the source
  * data for the duration of the operation.
  * This could be satisfied by the caller, holding a lock during the

Wording:
  * operation's call, or by the source data being
  * immutable, or by the source data not being shared across threads.
I would write:
  * operation's call, by immutable source data,
  * or by not sharing the source data across threads.

Missing commas:
  * character sequence contained in the string buffer does not exceed
  * character sequence, contained in the string buffer, does not exceed
Missing comma at 2 locations of:
      * object but does not synchronize on the source ({@code sb}).
      * object, but does not synchronize on the source ({@code sb}).

-Ulf

Am 25.06.2012 21:32, schrieb Jim Gish:
> Hopefully, this will address most, if not all of the suggestions made to date.
>
> ....Jim
>
> diff -r f37afaa214e9 src/share/classes/java/lang/StringBuffer.java
> --- a/src/share/classes/java/lang/StringBuffer.java    Mon Jun 25 14:22:42 2012 -0400
> +++ b/src/share/classes/java/lang/StringBuffer.java    Mon Jun 25 15:30:57 2012 -0400
> @@ -39,40 +39,38 @@
>   * that is consistent with the order of the method calls made by each of
>   * the individual threads involved.
>   * <p>
> - * The principal operations on a <code>StringBuffer</code> are the
> - * <code>append</code> and <code>insert</code> methods, which are
> + * The principal operations on a {@code StringBuffer} are the
> + * {@code append} and {@code insert} methods, which are
>   * overloaded so as to accept data of any type. Each effectively
>   * converts a given datum to a string and then appends or inserts the
>   * characters of that string to the string buffer. The
> - * <code>append</code> method always adds these characters at the end
> - * of the buffer; the <code>insert</code> method adds the characters at
> + * {@code append} method always adds these characters at the end
> + * of the buffer; the {@code insert} method adds the characters at
>   * a specified point.
>   * <p>
> - * For example, if <code>z</code> refers to a string buffer object
> - * whose current contents are "<code>start</code>", then
> - * the method call <code>z.append("le")</code> would cause the string
> - * buffer to contain "<code>startle</code>", whereas
> - * <code>z.insert(4, "le")</code> would alter the string buffer to
> - * contain "<code>starlet</code>".
> + * For example, if {@code z} refers to a string buffer object
> + * whose current contents are "{@code start}", then
> + * the method call {@code z.append("le")} would cause the string
> + * buffer to contain "{@code startle}", whereas
> + * {@code z.insert(4, "le")} would alter the string buffer to
> + * contain "{@code starlet}".
>   * <p>
> - * In general, if sb refers to an instance of a <code>StringBuffer</code>,
> - * then <code>sb.append(x)</code> has the same effect as
> - * <code>sb.insert(sb.length(), x)</code>.
> + * In general, if sb refers to an instance of a {@code StringBuffer},
> + * then {@code sb.append(x)} has the same effect as
> + * {@code sb.insert(sb.length(), x)}.
>   * <p>
>   * Whenever an operation occurs involving a source sequence (such as
> - * appending or inserting from a source sequence) this class synchronizes
> + * 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.
> + * {@code StringBuffer} is designed to be safe to use
> + * concurrently from multiple threads, but if the source data passed
> + * to the constructor  or to the {@code append()}, or {@code insert()}
> + * operations is shared across threads, the calling code must be ensure
> + * 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.
> + * operation's call, or by the source data being
> + * immutable, or by the source data not being 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
> @@ -112,8 +110,8 @@
>       * the specified initial capacity.
>       *
>       * @param      capacity  the initial capacity.
> -     * @exception  NegativeArraySizeException  if the <code>capacity</code>
> -     *               argument is less than <code>0</code>.
> +     * @exception  NegativeArraySizeException  if the {@code capacity}
> +     *               argument is less than {@code 0}.
>       */
>      public StringBuffer(int capacity) {
>          super(capacity);
> @@ -122,10 +120,10 @@
>      /**
>       * Constructs a string buffer initialized to the contents of the
>       * specified string. The initial capacity of the string buffer is
> -     * <code>16</code> plus the length of the string argument.
> +     * {@code 16} plus the length of the string argument.
>       *
>       * @param   str   the initial contents of the buffer.
> -     * @exception NullPointerException if <code>str</code> is <code>null</code>
> +     * @exception NullPointerException if {@code str} is {@code null}
>       */
>      public StringBuffer(String str) {
>          super(str.length() + 16);
> @@ -134,16 +132,16 @@
>
>      /**
>       * Constructs a string buffer that contains the same characters
> -     * as the specified <code>CharSequence</code>. The initial capacity of
> -     * the string buffer is <code>16</code> plus the length of the
> -     * <code>CharSequence</code> argument.
> +     * as the specified {@code CharSequence}. The initial capacity of
> +     * the string buffer is {@code 16} plus the length of the
> +     * {@code CharSequence} argument.
>       * <p>
> -     * If the length of the specified <code>CharSequence</code> is
> +     * If the length of the specified {@code CharSequence} is
>       * less than or equal to zero, then an empty buffer of capacity
> -     * <code>16</code> is returned.
> +     * {@code 16} is returned.
>       *
>       * @param      seq   the sequence to copy.
> -     * @exception NullPointerException if <code>seq</code> is <code>null</code>
> +     * @exception NullPointerException if {@code seq} is {@code null}
>       * @since 1.5
>       */
>      public StringBuffer(CharSequence seq) {
> @@ -264,10 +262,10 @@
>       * the new character sequence is equal to the character at index <i>k</i>
>       * in the old character sequence, if <i>k</i> is less than <i>n</i>;
>       * otherwise, it is equal to the character at index <i>k-n</i> in the
> -     * argument <code>sb</code>.
> +     * argument {@code sb}.
>       * <p>
> -     * This method synchronizes on <code>this</code> (the destination)
> -     * object but does not synchronize on the source (<code>sb</code>).
> +     * This method synchronizes on {@code this} (the destination)
> +     * object but does not synchronize on the source ({@code sb}).
>       *
>       * @param   sb   the <tt>StringBuffer</tt> to append.
>       * @return  a reference to this object.
> @@ -280,10 +278,10 @@
>
>
>      /**
> -     * Appends the specified <code>CharSequence</code> to this
> +     * Appends the specified {@code CharSequence} to this
>       * sequence.
>       * <p>
> -     * The characters of the <code>CharSequence</code> argument are appended,
> +     * The characters of the {@code CharSequence} argument are appended,
>       * in order, increasing the length of this sequence by the length of the
>       * argument.
>       *
> @@ -291,12 +289,12 @@
>       * invocation of this.append(s, 0, s.length());
>       *
>       * <p>This method synchronizes on this (the destination)
> -     * object but does not synchronize on the source (<code>s</code>).
> +     * object but does not synchronize on the source ({@code s}).
>       *
> -     * <p>If <code>s</code> is <code>null</code>, then the four characters
> -     * <code>"null"</code> are appended.
> +     * <p>If {@code s} is {@code null}, then the four characters
> +     * {@code "null"} are appended.
>       *
> -     * @param   s the <code>CharSequence</code> to append.
> +     * @param   s the {@code CharSequence} to append.
>       * @return  a reference to this object.
>       * @since 1.5
>       */
>
>
>
>