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

[Andrew Leonard]

Hi Alan,
I've thought about this some more, and you're right there are some edge 
cases where I think we really need to synchronize the start/stopListening. 
My initial thought the transportService would manage it, is flawed. For 
example in the potential situation where  dynamic port=0 allocation is 
used, and the transportService allocates ports, and a current thread using 
port A calls stopListening(), lets say that gets as far through that 
method as completing the transportService.stopListening() but not as far 
as the listenMap.remove(args), and then gets paused. Another thread calls 
startListening() and the transportService allocates the same port A to it 
and it completes the listenMap.put(args). The other thread then resumes 
and calls listenMap.remove(args), then removing the in-use port A of the 
other thread from the listenMap, thus causing an exception in accept() or 
stopListening()... and leaving a port in-use...

ConnectorImpl manages the defaultArguments map, which is an unaltered 
insertion-ordered linked map after object contruction. After construction 
it is purely for default arguments access, so in theory does not need 
synchronization, but all it would take is a scenario to be added for 
example to change a "default arg". So I think for safety making it 
Collections.synchronizedMap(new LinkedHashMap()); I think would make 
sense.

I shall work on a webrev.03 with these updates and perform some tests on 
xLinux, Win64 and mac64.

Thanks again
Andrew

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




From:   Alan Bateman <Alan.Bateman at oracle.com>
To:     Andrew Leonard <andrew_m_leonard at uk.ibm.com>
Cc:     serviceability-dev at openjdk.java.net
Date:   12/06/2019 10:54
Subject:        Re: RFR JDK-8225474: JDI connector accept fails "Address 
already in use" with concurrent listeners



On 10/06/2019 09:30, Andrew Leonard wrote:
Thanks for the feedback everyone. 

Hi Alan, yes I can put some cycles into digging deeper with this one, as 
you point out there's probably more that needs doing. 
As Alex found running the new testcase 400 times it failed once still, 
interestingly during initial startListening() rather than the accept() 
where it was failing before. 
On a general issue of "thread-safety", I had a good look through the spec 
docs and architecture docs and couldn't find any specification of 
thread-safety(or not), should it be inferred if the docs don't 
specifically state "thread-safe" that it should be assumed as "not-thread 
safe"? 
Right, Connectors aren't specified to be thread safe so any test that 
assumes otherwise is an issue (there may be a second issue here with a 
test using a fixed TCP port, it sounds like you might be running into both 
issues).

I've no objection to changing GenericListeningConnector to be thread safe. 
Also no issue with exploring the impact of a spec change too. Although 
Connectors (and transports) are pluggable, there probably aren't too many 
implementations outside of the JDK so you might have some scope to go 
beyond recommending that implementations be thread safe.

I've read your other mails where you've done some exploration into 
startListening/stopListening but I think there is more to do. Minimally I 
think startListening will need synchronization, alternatively changed to 
use putIfAbsent and stop the transport listening if it looses the race. 
There is also an issue with stopListening. It requires looking at the 
super class too because ConnectorImpl is not thread safe, esp. the 
defaultArguments which is a separate mutable map to listenMap.

-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/20190612/a73a78fe/attachment.html>