commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Henri Yandell <flame...@gmail.com>
Subject Re: svn commit: r899895 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/Range.java
Date Sat, 16 Jan 2010 19:51:15 GMT
On Sat, Jan 16, 2010 at 4:08 AM, sebb <sebbaz@gmail.com> wrote:
> On 16/01/2010, bayard@apache.org <bayard@apache.org> wrote:
>> Author: bayard
>>  Date: Sat Jan 16 07:58:11 2010
>>  New Revision: 899895
>>
>>  URL: http://svn.apache.org/viewvc?rev=899895&view=rev
>>  Log:
>>  Adding String lazy caching
>>
>>  Modified:
>>     commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/Range.java
>>
>>  Modified: commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/Range.java
>>  URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/Range.java?rev=899895&r1=899894&r2=899895&view=diff
>>  ==============================================================================
>>  --- commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/Range.java
(original)
>>  +++ commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/Range.java
Sat Jan 16 07:58:11 2010
>>  @@ -28,13 +28,33 @@
>>   */
>>   public final class Range<T> implements Serializable {
>>
>>  +    /**
>>  +     * Required for serialization support.
>>  +     *
>>  +     * @see java.io.Serializable
>>  +     */
>>      private static final long serialVersionUID = 1L;
>>
>>  +    /**
>>  +     * The ordering scheme used in this range.
>>  +     */
>>      private final Comparator<T> comparator;
>>  +
>>  +    /**
>>  +     * The minimum value in this range (inclusive).
>>  +     */
>>      private final T minimum;
>>  +    /**
>>  +     * The maximum value in this range (inclusive).
>>  +     */
>>      private final T maximum;
>>
>>      /**
>>  +     * Cached output toString (class is immutable).
>>  +     */
>>  +    private transient String toString = null;
>>  +
>>  +    /**
>>       * <p>Constructs a new <code>Range</code> using the specified
>>       * element as both the minimum and maximum in this range.</p>
>>       * <p>The range uses the natural ordering of the elements to
>>  @@ -320,16 +340,18 @@
>>       */
>>      @Override
>>      public String toString() {
>>  -        StringBuilder buf = new StringBuilder(32);
>>  -        buf.append("Range[");
>>  -        buf.append(this.minimum);
>>  -        buf.append(',');
>>  -        buf.append(this.maximum);
>>  -        buf.append(']');
>>  -        return buf.toString();
>>  +        if (toString == null) {
>>  +            StringBuilder buf = new StringBuilder(32);
>>  +            buf.append("Range[");
>>  +            buf.append(this.minimum);
>>  +            buf.append(',');
>>  +            buf.append(this.maximum);
>>  +            buf.append(']');
>>  +            toString = buf.toString();
>>  +        }
>>  +        return toString;
>
> There's some doubt as to whether fetching the toString class variable
> twice is thread-safe.
>
> See long thread in Harmony [1] and [2]
>
> [1] http://harmony.markmail.org/message/ud3c6ngyfjeymdga?q=data+reordering+hashCode
> [2] http://jeremymanson.blogspot.com/2008/12/benign-data-races-in-java.html
>
> Also HARMONY-6404.
>
> It would be safer to use a temp. local String and return that.

Changed. LANG-481 had indicated toString being happy, but I suspect
that was the original report rather than the later issue.

Hen

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message