RFR: 691423 - (str) Missing synchronization in java.lang.String#contentEquals(CharSequence)

[Mike Duigou]

This would appear to be source compatible but not binary compatible. I don't believe we can remove the contentsEqual(StringBuffer) overload. Code compiled against the existing interface would fail to find the CharSequence interface at runtime and fail.

I believe it would be reasonable to add locking on StringBuffer into the contentsEqual(CharSequence) overload. This would make the behaviour consistent between the two invocations which is important because it will not always be possible for developers to tell which overload is being selected.

Mike

On Jul 26 2012, at 13:12 , Jim Gish wrote:

> Summary:  currently String.contentEquals( StringBuffer sb ) synchronizes on sb, but String.contentEquals( CharSequence cs ) does not sync when cs is a StringBuffer
> Proposed change:  remove contentEquals( StringBuffer ) and have contentEquals( CharSequence ) do the checking and synchronize if the cs is a StringBuffer.
> Note: I expect this will require CCC approval because of the removal of contentEquals( StringBuffer ).
> 
> Thanks,
>   Jim
> 
> Proposed patch:
> diff --git a/src/share/classes/java/lang/String.java b/src/share/classes/java/lang/String.java
> --- a/src/share/classes/java/lang/String.java
> +++ b/src/share/classes/java/lang/String.java
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 1994, 2010, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 1994, 2012, Oracle and/or its affiliates. All rights reserved.
>  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>  *
>  * This code is free software; you can redistribute it and/or modify it
> @@ -984,30 +984,26 @@
>         return false;
>     }
> 
> -    /**
> -     * Compares this string to the specified {@code StringBuffer}.  The result
> -     * is {@code true} if and only if this {@code String} represents the same
> -     * sequence of characters as the specified {@code StringBuffer}.
> -     *
> -     * @param  sb
> -     *         The {@code StringBuffer} to compare this {@code String} against
> -     *
> -     * @return  {@code true} if this {@code String} represents the same
> -     *          sequence of characters as the specified {@code StringBuffer},
> -     *          {@code false} otherwise
> -     *
> -     * @since  1.4
> -     */
> -    public boolean contentEquals(StringBuffer sb) {
> -        synchronized (sb) {
> -            return contentEquals((CharSequence) sb);
> +    private boolean contentEquals(AbstractStringBuilder sb) {
> +        char v1[] = value;
> +        char v2[] = sb.getValue();
> +        int i = 0;
> +        int n = value.length;
> +        while (n-- != 0) {
> +            if (v1[i] != v2[i]) {
> +                return false;
> +            }
> +            i++;
>         }
> +        return true;
>     }
> 
>     /**
> -     * Compares this string to the specified {@code CharSequence}.  The result
> -     * is {@code true} if and only if this {@code String} represents the same
> -     * sequence of char values as the specified sequence.
> +     * Compares this string to the specified {@code CharSequence}.  The
> +     * result is {@code true} if and only if this {@code String} represents the
> +     * same sequence of char values as the specified sequence. Note that if the
> +     * {@code CharSequence} is a {@code StringBuffer} then the method
> +     * synchronizes on it.
>      *
>      * @param  cs
>      *         The sequence to compare this {@code String} against
> @@ -1023,16 +1019,13 @@
>             return false;
>         // Argument is a StringBuffer, StringBuilder
>         if (cs instanceof AbstractStringBuilder) {
> -            char v1[] = value;
> -            char v2[] = ((AbstractStringBuilder) cs).getValue();
> -            int i = 0;
> -            int n = value.length;
> -            while (n-- != 0) {
> -                if (v1[i] != v2[i])
> -                    return false;
> -                i++;
> +            if (cs instanceof StringBuffer) {
> +                synchronized(cs) {
> +                   return contentEquals((AbstractStringBuilder)cs);
> +                }
> +            } else {
> +                return contentEquals((AbstractStringBuilder)cs);
>             }
> -            return true;
>         }
>         // Argument is a String
>         if (cs.equals(this))
> 
> 
> 
> -- 
> 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
>