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