Re: RFR JDK-8225474: JDI connector accept fails "Address already in use" with concurrent listeners
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_leon...@uk.ibm.com From: David Holmes To: Alan Bateman , "serguei.spit...@oracle.com" , Andrew Leonard , serviceability-dev@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.spit...@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
Re: RFR JDK-8225474: JDI connector accept fails "Address already in use" with concurrent listeners
Thanks Alex, I have changed the testcase to allocate ports which makes better sense: http://cr.openjdk.java.net/~aleonard/8225474/webrev.02/ Cheers Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd internet email: andrew_m_leon...@uk.ibm.com From: Alex Menkov To: Andrew Leonard Cc: serviceability-dev@openjdk.java.net Date: 10/06/2019 20:43 Subject:Re: RFR JDK-8225474: JDI connector accept fails "Address already in use" with concurrent listeners On 06/10/2019 02:07, Andrew Leonard wrote: > Hi Alex, > Thanks for trying this out, it's possible this failure is an issue with > my testcase, as I repeat the 10 thread test loop 10 times and I have a > 30 second timeout which potentially might overrun if a connection takes > time to terminate. I could avoid that simply by using different ports > for each run, equally it could also be an issue as pointed out by Alan. The test uses fixed ports, so it can interfere with other processes. You can use port "0" which automatically selects free port. connector.startListening returns actual port assigned for the connection. > Thanks > Andrew > > Andrew Leonard > Java Runtimes Development > IBM Hursley > IBM United Kingdom Ltd > internet email: andrew_m_leon...@uk.ibm.com > > > > > From: Alex Menkov > To: Andrew Leonard , > serviceability-dev@openjdk.java.net > Date: 07/06/2019 22:19 > Subject: Re: RFR JDK-8225474: JDI connector accept fails "Address > already in use" with concurrent listeners > > > > > Hi Andrew, > > The fix looks good in general. > And I can be the sponsor. > I've tested the fix and got 1 test failure (of 400 runs) on Win-x64 and > the log looks the same. > > java.lang.RuntimeException: Failed to accept connector connection > at JdwpConcurrentAttachTest.main(JdwpConcurrentAttachTest.java:74) > at > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native > Method) > at > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) > at > java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > at java.base/java.lang.reflect.Method.invoke(Method.java:567) > at > com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127) > at java.base/java.lang.Thread.run(Thread.java:830) > Caused by: java.net.BindException: Address already in use: bind > at java.base/sun.nio.ch.Net.bind0(Native Method) > at java.base/sun.nio.ch.Net.bind(Net.java:465) > at java.base/sun.nio.ch.Net.bind(Net.java:457) > at java.base/sun.nio.ch.NioSocketImpl.bind(NioSocketImpl.java:643) > at java.base/java.net.ServerSocket.bind(ServerSocket.java:359) > at java.base/java.net.ServerSocket.bind(ServerSocket.java:313) > at > jdk.jdi/com.sun.tools.jdi.SocketTransportService.startListening(SocketTransportService.java:300) > at > jdk.jdi/com.sun.tools.jdi.SocketTransportService.startListening(SocketTransportService.java:310) > at > jdk.jdi/com.sun.tools.jdi.GenericListeningConnector.startListening(GenericListeningConnector.java:117) > at > jdk.jdi/com.sun.tools.jdi.SocketListeningConnector.startListening(SocketListeningConnector.java:86) > at JdwpConcurrentAttachTest.attachTest(JdwpConcurrentAttachTest.java:111) > at JdwpConcurrentAttachTest.run(JdwpConcurrentAttachTest.java:93) > > So looks like the fix doesn't fix the issue completely. > > --alex > > On 06/07/2019 07:24, Andrew Leonard wrote: >> Hi, >> Please can I request a sponsor and review of this patch for JDK-8225474. >> A problem that we have seen intermittently with JDI connector stress >> tests for quite a while that is caused by an non-thread safe HashMap in >> the connector class. I've created a standalone testcase that reproduces >> it reliably. >> https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Ealeonard_8225474_webrev.00=DwIC-g=jf_iaSHvJObTbx-siA1ZOg=NaV8Iy8Ld-vjpXZFDdTbgGlRTghGHnwM75wUPd5_NUQ=OaT9vhOgj2gp0DQBwJgmItAjmzP-GsUEkGJAVbN9ZCE=EQwT-4TnAIqImG1-KTOT803z1DRjmTTeMtNV-9S9-6M= >> >> Thanks >> Andrew >> >> >> Andrew Leonard >> Java Runtimes Development >> IBM Hursley >> IBM United Kingdom Ltd >> internet email: andrew_m_leon...@uk.ibm.com >> >> 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 > > > > > 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 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
