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

[Daniel Fuchs]

On Fri, 24 May 2024 14:41:47 GMT, Mark Sheppard <msheppar at openjdk.org> wrote:

>> 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
> }

So we could do:

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

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

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