jcktej / jdiameter

Automatically exported from code.google.com/p/jdiameter
0 stars 0 forks source link

TCPTransportClient.sendMessage() does not write all bytes when the pipe is full, and the rest of the message is discarded #41

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
In sendMessage(),

      rc = socketChannel.write(bytes);

the code assumes that all the bytes have been written out. However, if the 
socket is not available for writing, only a portion of the bytes (or none at 
all) will in fact be written, since socketChannel is non blocking channel.

This can occur when the other side not reading/processing messages fast enough 
and the TCP send window becomes full. This is the circumstance under which I 
encountered this issue. Messages that I was sending were not being fully sent. 
A simple fix such as : 

     rc = 0;
     while(rc<bytes.array().length)rc+= socketChannel.write(bytes);

resolved the issue.

For reference, here is the sendMessage() method :
--------------------------

public void sendMessage(ByteBuffer bytes) throws IOException {
    if (logger.isDebugEnabled()) {
      logger.debug("About to send a byte buffer of size [{}] over the TCP nio socket [{}]", bytes.array().length, socketDescription);
    }
    int rc;
    lock.lock();   
    try {
      rc = socketChannel.write(bytes);
    }
    catch (Exception e) {
      logger.debug("Unable to send message", e);
      throw new IOException("Error while sending message: " + e);
    }
    finally {
      lock.unlock();
    }
    if (rc == -1) {
      throw new IOException("Connection closed");
    }
    if (logger.isDebugEnabled()) {
      logger.debug("Sent a byte buffer of size [{}] over the TCP nio socket [{}]", bytes.array().length, socketDescription);
    }
  }

Original issue reported on code.google.com by int...@gmail.com on 7 Mar 2013 at 9:33

GoogleCodeExporter commented 9 years ago

I came across this bug also whilst load testing the JDiameter stack. It is a 
big problem for running the stack in proxy mode as it causes a leak in the 
message router table. The knock on consequence is that the proxy falls over in 
a heap.

The suggested fix is problem okay but the write should really be integrated in 
the select loop where the socket reads are done (see 
http://rox-xmlrpc.sourceforge.net/niotut/ for example of the select with 
SelectionKey.OP_WRITE).

Original comment by dpalmer...@gmail.com on 30 Jun 2014 at 11:08

GoogleCodeExporter commented 9 years ago
Indeed, this is a severe problem. We'll probably follow with int999 suggestion 
for now, although in the long run we should do the right thing and implement it 
using SelectionKey.OP_WRITE as suggested by dpalmer895 and nicely explained in 
the referenced tutorial.

int999: you seem to have re-enabled the locks, any reason for that ?

dpalmer895: feel free to provide a patch if you feel like, we'd be more than 
happy to help you with it if needed, review it and integrate.

(Adding PCB and Richard Good to discussion as they have made some contributions 
to the non-blocking NIO part)

Original comment by brainslog on 8 Jul 2014 at 1:34

GoogleCodeExporter commented 9 years ago
Issue 55 has been merged into this issue.

Original comment by brainslog on 8 Jul 2014 at 1:36

GoogleCodeExporter commented 9 years ago
From Richard and Paul - 

A simple fix such as : 

     rc = 0;
     while(rc<bytes.array().length)rc+= socketChannel.write(bytes);

resolved the issue.

We are happy with this fix and have it implemented and running fine on our 
platforms.

Thanks for the patch!

Original comment by richard....@smilecoms.com on 16 Jul 2014 at 1:45

GoogleCodeExporter commented 9 years ago
This issue was updated by revision 08e54137c429.

Fixed according to suggestion.

Original comment by brainslog on 1 Aug 2014 at 12:33