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_leon...@uk.ibm.com From: Alan Bateman <alan.bate...@oracle.com> To: Andrew Leonard <andrew_m_leon...@uk.ibm.com> Cc: serviceability-dev@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
