RFR: 8237388: serviceability/dcmd/framework/VMVersionTest.java fails with connection refused error.

[Alex Menkov]

On Thu, 3 Jun 2021 21:44:32 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> nsk test framework for testing jdi/jdwp assumes that the test launch debuggee first and then creates IOPipe to communicate with debuggee (debugger listens, debugee connects to the IOPipe socket).
>> This makes possible failure when debuggee tries to connect before debugger starts listening.
>> The fix changes logic of the tests to start listening on IOPipe socket before launch debuggee.
>> Other tests which use IOPipe/SocketIOPipe and have the same issue (there are a lot of them) will be fixes as separate issues.
>> 
>> Couple details about the fix:
>> - debugee.getPipeServerSocket() just calls binder.getPipeServerSocket(), returned ServerSocket can be changed only by DebugeeBinder.prepareForPipeConnection which is called by some tests;
>> - connectingProcess logic is broken. The only place it's used is BasicSocketConnection.checkConnectingProcess, which is called from SocketConnection.continueAttach. But continueAttach is called from debuggee code (listening == false) while connectingProcess is set only from debugger code (listening == true). So it didn't work and I dropped it.
>> 
>> Testing: all tests which use IOPipe/SocketIOPipe classes:
>> - test/hotspot/jtreg/serviceability/dcmd/framework
>> - test/hotspot/jtreg/vmTestbase/nsk/jdi
>> - test/hotspot/jtreg/vmTestbase/nsk/jdwp
>
> test/hotspot/jtreg/vmTestbase/nsk/share/jpda/SocketIOPipe.java line 275:
> 
>> 273:         if (listening) {
>> 274:             // listenerThread == null means the test is not updated yet
>> 275:             // to start IOPipe listening before launching debuggee.
> 
> I like this workaround for non-updated tests. This spot can be simplified after all tests got updated.

Yes, the plan is to put assert here later.

> test/hotspot/jtreg/vmTestbase/nsk/share/jpda/SocketIOPipe.java line 279:
> 
>> 277:                 // start listening and accept connection on the current thread
>> 278:                 listenerThread = new ListenerThread();
>> 279:                 listenerThread.run();
> 
> It is not clear, why `listenerThread.run()` is used instead of `listenerThread.start()` as at the line 257.

This is part of the workaround for non-updated tests - the tests should work the same way they did.
start() would start new thread, so listening starts a bit later and we have more chances to get "connection refused" error.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4235