On Wed, 20 Jan 2021 13:40:51 GMT, Mark Sheppard <mshep...@openjdk.org> wrote:

>> LGTM. Thanks Patrick for taking this on!
>
> This response maybe a bit of overkill or overthinking, but bear with me and 
> apologies in advance!!
> I think some of the additions have a few (theoretical) consequences - the 
> read buffer and write buffer asserts cannot be assumed to be true as this, 
> afaik, is not a datagram communication scenario ?? As such read and writes 
> are not atomic.
> 
> Additionally they (may) distract from the main semantics of the test, that 
> is, the read NAPI remain constant across reads.
> 
> The test has been modified to assert some conditions on the read and write 
> buffer. These asserts, in theory, can not be assumed to hold true. Consider 
> that the write returns a Future and will inform the writer how many bytes 
> have been written. So writes are not atomic.
> Additionally, as such,  a read may not actually read the number of bytes 
> written by the writer, 
> due to the underlying semantics of the supporting protocol (TCP/IP). If it 
> was UDP as the underlying protocol then the read/write assertions would hold.
> Because it is a co-located writer/reader test, and the size of the data 
> transfer is small, the likelihood of any variation between the write and read 
> is small.
> So, do you really need to do the read/write buffer checks. Adding the get() 
> method to the Future returned by the read should be sufficient to obviate the 
> ReadPendingException
> 
> One other minor point: Is the assignment tempID = clientID;  needed for each 
> iteration of the loop. The clientID should be constant across all reads.  If 
> tempID was renamed originalClientID and assigned in the initialRun block
> 
>                      if (initialRun) {
>                             assertTrue(clientID >= 0, 
> "AsynchronousSocketChannel: Receiver");
>                             initialRun = false;
>                             originalClientID = clientID
>                         } else {
>                             assertEquals(clientID, originalClientID);
>                         }

Hi Mark,

> The test has been modified to assert some conditions on the read and write 
> buffer. These asserts, in theory, can not be assumed to hold true. Consider 
> that the write returns a Future and will inform the writer how many bytes 
> have been written. So writes are not atomic.
> Additionally, as such, a read may not actually read the number of bytes 
> written by the writer,
> due to the underlying semantics of the supporting protocol (TCP/IP). If it 
> was UDP as the underlying protocol then the read/write assertions would hold.

My understanding is that read will read a full datagram (or nothing) and write 
will write a full datagram (or nothing). Indeed a datagram is the smallest unit 
that can be read or written on a UDP channel. Two calls to write() will produce 
two datagrams. Two calls to read() will attempt to read two datagrams.

best regards,

-- daniel

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

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

Reply via email to