On Wed, 20 Jan 2021 13:40:51 GMT, Mark Sheppard <[email protected]> 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
