SctpChannel.send() return value ignored

[dpalmer895]

In AssociationImpl.java the return value of send is ignored:

protected void write(SelectionKey key) {

	try {			
		// TODO Do we need to synchronize ConcurrentLinkedQueue?
		// synchronized (this.txQueue) {
		if (!txQueue.isEmpty()) {
			while (!txQueue.isEmpty()) {
				// Lets read all the messages in txQueue and send

				PayloadData payloadData = txQueue.poll();

				if (logger.isDebugEnabled()) {
					logger.debug(String.format("Tx : Ass=%s %s", this.name, payloadData));
				}

				if (this.ipChannelType == IpChannelType.SCTP) {
					int seqControl = payloadData.getStreamNumber();

					if (seqControl < 0 || seqControl >= this.associationHandler.getMaxOutboundStreams()) {
						try {
							// TODO : calling in same Thread. Is this ok? or
							// dangerous?
							this.associationListener.inValidStreamId(payloadData);
						} catch (Exception e) {

						}
						
						continue;
					}

					msgInfo = MessageInfo.createOutgoing(this.peerSocketAddress, seqControl);
					msgInfo.payloadProtocolID(payloadData.getPayloadProtocolId());
					msgInfo.complete(payloadData.isComplete());
					msgInfo.unordered(payloadData.isUnordered());
				}

				try
				{
					this.doSend(payloadData.getByteBuf());
				}
				finally {
					payloadData.releaseBuffer();
				}					

			}// end of while
		}

		if (txQueue.isEmpty()) {
			// We wrote away all data, so we're no longer interested
			// in writing on this socket. Switch back to waiting for
			// data.
			key.interestOps(SelectionKey.OP_READ);
		}

	} catch (IOException e) {
		this.ioErrors++;
		logger.error(String.format(
				"IOException while trying to write to underlying socket for Association=%s IOError count=%d",
				this.name, this.ioErrors), e);

		if (this.ioErrors > this.management.getMaxIOErrors()) {
			// Close this socket
			this.close();

			// retry to connect after delay
			this.scheduleConnect();
		}
	}// try-catch
}

private int doSend(ByteBuf buffer) throws IOException {
	if (this.ipChannelType == IpChannelType.SCTP)
		return this.doSendSctp(buffer);
	else
		return this.doSendTcp(buffer);
}

private int doSendSctp(ByteBuf buffer) throws IOException {
	return this.socketChannelSctp.send(buffer.nioBuffer(), msgInfo);
}

private int doSendTcp(ByteBuf buffer) throws IOException {
	return this.socketChannelTcp.write(buffer.nioBuffer());
}

You can see that the write method calls doSend() but ignores the return value. From what I could see the socket is configured in non blocking mode. In this case where the underlying output buffer is full the SctpChannel.send() method would return 0 indicating no bytes of the message were written. However the write() method doesn't check the return value and just assumes that the message was sent successfully (i.e. message not sent and therefore lost).

[gsaslis]

Thanks for raising this @dpalmer895 !
Were you able to figure this out by any chance?

@ammendonca any thoughts?

[dpalmer895]

I have implemented the following fix in the "protected void write(SelectionKey key)" method:

 int toBeSent = payloadData.getDataLength();
 if (toBeSent > 0)

{
// David Palmer - 26/01/2018
// I am not 100% certain but my interpretation is that SctpChanel API is that
// either all the data will be sent or none of it in the case where there is not
// enough space in the underlying buffer.
// In the case it is not sent push the message back onto the send queue
//
int bytesSent = this.doSend(payloadData.getByteBuf());
if (bytesSent == 0)
{
txQueue.add(payloadData);
}
else if (bytesSent != toBeSent)
{
logger.error("SctpChanell.send() returned a value other than the size of the input buffer or zero");
}
}

This was a quick fix to stop the loss of the message when the buffer is full. It adds the message to the end of the pending message queue so it will be sent out of order but at least it is not lost.

[gsaslis]

@dpalmer895 awesome, thanks for taking the time to share back! 🎉

@ammendonca do you think you would have some time to review this any time soon?

[ammendonca]

@vetss or @knosach are more suited to review this.

[mon205]

I am facing the same issue also. Is there an updated version with fix?

[alpercoskun]

I think this problem is only for non-netty implementation ?

[nhanth87]

right,
only for non-netty but the netty version is not tested so I don't recommend you to switch to netty version but apply the fix and build your own version

[alpercoskun]

netty version is not tested ? what does it mean ? jss7 uses netty version for a long time according to history...

https://www.telestax.com/blog/telscale-jss7-performance-improvements/

[chuquanghopbk]

I encouter the same error, have anyone resolve it :(