RFR: 7903730: Enhance the agentvm to be tolerant to unexpected connection on the port the Agent listens on for handshake with the AgentServer [v3]

[Mark Sheppard]

On Thu, 23 May 2024 16:32:37 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> right ... asserts are conditional on being enabled ... if the condition is a valid test then handled it properly and gracefully in the code not with a conditional abort
>
> Hello Mark, the purpose of the assert is to catch any programming mistake/error. Where as the `if` block is to verify that the runtime handshake did indeed succeed.
> 
> At runtime, it is possible that an unexpectedly connected peer might send data that is less than what we expect in a handshake - the if block is meant to handle that case and close such connections.
> 
> The assert and the if serve two different purposes in this code.

JP, yes understood
I should have been more direct and precise in my comment i.e. 
the assert is not needed — restructure the if statement.

The condition can’t occur and if it did occur then what would that mean?
Is there a problem with either the code generated by javac ? or 
that the java runtime has a problem? or the return from InputStream.read is greater than expected, 
thus the InputStream has a problem?

An AgentServer is (nearly) always launched with “-ea” option — only if jtreg is launched from command line without -ea will it not have asserts enabled.

Looking at the current logic, rather than testing all negative cases, it can be restructured with

if (totalRead != JTREG_AGENT_HANDSHAKE_MAGIC.length) {
   // problem
} else {
   // check for connect message
}

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

PR Review Comment: https://git.openjdk.org/jtreg/pull/195#discussion_r1613592394