RFR JDK-8225474: JDI connector accept fails "Address already in use" with concurrent listeners

[Andrew Leonard]

So good point, what does thread-safety mean here:

I've gone back around my change and am now satisfied just changing to use 
ConcurrentHashMap is sufficient for what is required here. The 
GenericListenerConnector and it's sub-class are essentially a wrapper 
class to the underlying TransportService object. It essentially marshalls 
the listening requests on various addresses/ports through to the 
transportService. The addr:port uniqueness/clashes is ultimately delegated 
to the transportService who really knows what ports are allocated, and as 
long as the GenericListenerConnector only put's listener's into its map 
upon a thread's successful transportService.startListening() and removes 
them upon a successful transportService.stopListening(), that should work, 
hence why I have left stopListening() method as it was (in webrev.02). The 
real requirement of GenericListenerConnector from a thread-safety aspect 
is that it safely stores/manages it's map of listeners, which is where the 
bug was, in that it needed a ConcurrentHashMap. Thus get/puts.. don't 
corrupt the map.

I am thus happy with my webrev.02, which is as .00 but i've updated the 
testcase to allocate ports using port=0.
    http://cr.openjdk.java.net/~aleonard/8225474/webrev.02/
I have now run my testcase x200 with no failure.
I have also run the suggested jshell tests numerous times all passing:
   jtreg -concurrency:12 -v1 langtools/test/jdk/jshell 

So I am quite happy with it as it stands, i've thrashed the startListening 
concurrency quite hard on xLinux and it now seems robust.
I am going to run some stress tests on some other platforms tomorrow, 
Win64 and Mac if I can.

Is one of the JDI SME's able to review this please?

Thanks
Andrew

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
internet email: andrew_m_leonard at uk.ibm.com 




From:   David Holmes <david.holmes at oracle.com>
To:     Alan Bateman <Alan.Bateman at oracle.com>, 
"serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com>, Andrew Leonard 
<andrew_m_leonard at uk.ibm.com>, serviceability-dev at openjdk.java.net
Date:   09/06/2019 14:30
Subject:        Re: RFR JDK-8225474: JDI connector accept fails "Address 
already in use" with concurrent listeners



+1 from me.

What thread-safety should mean here needs to be defined.

David

On 8/06/2019 7:11 pm, Alan Bateman wrote:
> On 08/06/2019 05:53, serguei.spitsyn at oracle.com wrote:
>> Hi Andrew,
>>
>> It looks good to me.
>> Thank you for your investigation and taking care about this!
> I think this one needs further analysis as changing listenMap to be a 
> CHM does not make this connector thread safe and doesn't address a 
> number of other places where the map is accessed without 
> synchronization. For example, startListening would need to be changed to 

> use `putIfAbsent`, stopListening would need to use `remove` rather than 
> get + remote. There are several others once you start looking at the 
> sub-classes, e.g. SocketListeningConnector.updateArgumentMapIfRequired.
> 
> At the API level, JDI does not require connectors to be thread safe. I 
> agree it is desirable for some of the connector implementations to be 
> thread safe, particularly ListeningConnectors as an obvious usage will 
> be for a listener thread to hand off a VirtualMachine to another thread.
> 
> Andrew - do you have cycles to investigate this further? I'm not opposed 

> to changing the map to be a CHM but I think it needs further work.
> 
> -Alan




Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190611/0f80c7c0/attachment-0001.html>