[GEODE-9825] Disparate socket-buffer-size Results in "IOException: Unknown header byte" and Hangs - ASF JIRA

XML

Word

Printable

JSON

Details

Type: Bug
Status: Closed
Priority: Major
Resolution: Fixed
Affects Version/s: 1.12.4, 1.15.0
Fix Version/s: 1.12.6, 1.13.5, 1.14.1, 1.15.0
Component/s: messaging
Labels:
- pull-request-available

Description

~~GEODE-9141~~ introduced a bug that causes IOException: "Unknown header byte..." and hangs if members are configured with different socket-buffer-size settings.

Reproduction

To reproduce this bug turn off TLS and set socket-buffer-size on sender to be 64KB and set socket-buffer-size on receiver to be 32KB. See associated PR for an example.

Analysis

In Connection.processInputBuffer(). When that method has read all the messages it can from the current input buffer, it then considers whether the buffer needs expansion. If it does then:

inputBuffer = inputSharing.expandReadBufferIfNeeded(allocSize);

Is executed and the method returns. The caller then expects to be able to write bytes into inputBuffer.

The problem, it seems, is that ByteBufferSharingInternalImpl.expandReadBufferIfNeeded() does not leave the the ByteBuffer in the proper state. It leaves the buffer ready to be read not written.

Before the changes for ~~GEODE-9141~~ were introduced, the line of code referenced above used to be this snippet in Connection.compactOrResizeBuffer(int messageLength) (that method has since been removed):

     // need a bigger buffer
    logger.info("Allocating larger network read buffer, new size is {} old size was {}.",
        allocSize, oldBufferSize);
    ByteBuffer oldBuffer = inputBuffer;
    inputBuffer = getBufferPool().acquireDirectReceiveBuffer(allocSize);    
    if (oldBuffer != null) {
      int oldByteCount = oldBuffer.remaining();
      inputBuffer.put(oldBuffer);
      inputBuffer.position(oldByteCount);
      getBufferPool().releaseReceiveBuffer(oldBuffer);
    }

Notice how this method leaves inputBuffer ready to be written to.

But the code inside ByteBufferSharingInternalImpl.expandReadBufferIfNeeded() is doing something like:

newBuffer.clear();
newBuffer.put(existing);
newBuffer.flip();
releaseBuffer(type, existing);
return newBuffer;

A solution (shown in the associated PR) is to do add logic after the call to expandReadBufferIfNeeded(allocSize) to leave the buffer in a writeable state:

inputBuffer = inputSharing.expandReadBufferIfNeeded(allocSize);
// we're returning to the caller (done == true) so make buffer writeable
inputBuffer.position(inputBuffer.limit());
inputBuffer.limit(inputBuffer.capacity());

Resolution

When this ticket is complete the bug will be fixed and P2PMessagingConcurrencyDUnitTest will be enhanced to test at least these combinations:

[security, sender/locator socket-buffer-size, receiver socket-buffer-size]

[TLS, (default), (default)] this is what the test currently does
[no TLS, 64 * 1024, 32 * 1024] new: this illustrates this bug
[no TLS, (default), (default)] new

We might want to mix in conserve-sockets true/false in there too while we're at it (the test currently holds it at true).