Re: RFR(S): 8155590: Dubious collection management in sun.net.www.http.KeepAliveCache

[Roger Riggs]

Hi,

Keep the synchronized, it will be low overhead since the Vector operations
are synchronized and in the same thread.
I think a CCE could occur during the iteration to find the entry when 
Vector.Itr.next() checks.

(It you want to more radical fix, replace the Vector with something more 
current.
It would be one less Vector).

Roger


On 10/16/17 2:33 AM, Langer, Christoph wrote:

Hi Vyom,

thanks for your feedback.

I’m not so sure about dropping “synchronized”. In the new remove 
method of ClientVector we are iterating ourself. If this is not done 
under synchronization, there is risk to run into a 
ConcurrentModificationException. But under the assumption that all 
access to ClientVector comes from synchronized methods of 
KeepAliveCache, one could argue to drop all synchronized modifiers for 
ClientVector, though.

Let’s wait for other opinions J

Best regards

Christoph

*From:*net-dev [mailto:net-dev-boun...@openjdk.java.net] *On Behalf Of 
*vyom tewari
*Sent:* Montag, 16. Oktober 2017 10:27
*To:* net-dev@openjdk.java.net
*Subject:* Re: RFR(S): 8155590: Dubious collection management in 
sun.net.www.http.KeepAliveCache

Hi Christoph,

Thanks for doing this, i think you don't need to synchronize the 
"remove(HttpClient h)".  This remove is get called from synchronize 
"remove (HttpClient h, Object obj)" and the underline data structure 
is which is java.util.Vector(ClientVector extends java.util.Stack) is 
also thread safe.

What do you think ?

Thanks,

Vyom

On Monday 16 October 2017 12:52 PM, Langer, Christoph wrote:

    Hi,

    Here is a proposal for a fix for bug 8155590. I already made this
    fix a while ago in our JDK clone and I’d like to contribute this.

    Bug: https://bugs.openjdk.java.net/browse/JDK-8155590

    Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8155590.0/
    <http://cr.openjdk.java.net/%7Eclanger/webrevs/8155590.0/>

    Please review.

    Thanks

    Christoph