You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@zookeeper.apache.org by Robin <rc...@163.com> on 2014/11/25 02:10:38 UTC

Possible logic error in ClientCnxnSocketNIO

Hi,
When SASL authentication is enabled, the ZooKeeper client will finally call ClientCnxnSocketNIO#sendPacket(Packet p) to send a packet to server:
@Override
void sendPacket(Packet p) throws IOException {
    SocketChannel sock = (SocketChannel) sockKey.channel();
    if (sock == null) {
        throw new IOException("Socket is null!");
    }
    p.createBB();
    ByteBuffer pbb = p.bb;
    sock.write(pbb);
}

One problem I can see is that the sock is non-blocking, so when the sock's output buffer is full(theoretically), only part of the Packet is sent out and the communication will break.

Thanks,
- Robin

Re:Re: Possible logic error in ClientCnxnSocketNIO

Posted by Robin <rc...@163.com>.
Sure. It's created: https://issues.apache.org/jira/browse/ZOOKEEPER-2091 

- Robin


At 2014-11-26 01:49:53, "Raúl Gutiérrez Segalés" <rg...@itevenworks.net> wrote:
>Hi Robin,
>
>On 24 November 2014 at 17:10, Robin <rc...@163.com> wrote:
>
>> Hi,
>> When SASL authentication is enabled, the ZooKeeper client will finally
>> call ClientCnxnSocketNIO#sendPacket(Packet p) to send a packet to server:
>> @Override
>> void sendPacket(Packet p) throws IOException {
>>     SocketChannel sock = (SocketChannel) sockKey.channel();
>>     if (sock == null) {
>>         throw new IOException("Socket is null!");
>>     }
>>     p.createBB();
>>     ByteBuffer pbb = p.bb;
>>     sock.write(pbb);
>> }
>>
>> One problem I can see is that the sock is non-blocking, so when the sock's
>> output buffer is full(theoretically), only part of the Packet is sent out
>> and the communication will break.
>>
>
>I think you are right, because that socket is set to non-blocking:
>
>https://github.com/apache/zookeeper/blob/trunk/src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java#L267
>
>So it looks like the sock.write call should be wrapped into something like:
>
>  while (pbb.remaining() > 0) {
>      sock.write(pbb);
>  }
>
>Do you mind filing a ticket for this? Thanks!
>
>
>-rgs

Re: Possible logic error in ClientCnxnSocketNIO

Posted by Raúl Gutiérrez Segalés <rg...@itevenworks.net>.
Hi Robin,

On 24 November 2014 at 17:10, Robin <rc...@163.com> wrote:

> Hi,
> When SASL authentication is enabled, the ZooKeeper client will finally
> call ClientCnxnSocketNIO#sendPacket(Packet p) to send a packet to server:
> @Override
> void sendPacket(Packet p) throws IOException {
>     SocketChannel sock = (SocketChannel) sockKey.channel();
>     if (sock == null) {
>         throw new IOException("Socket is null!");
>     }
>     p.createBB();
>     ByteBuffer pbb = p.bb;
>     sock.write(pbb);
> }
>
> One problem I can see is that the sock is non-blocking, so when the sock's
> output buffer is full(theoretically), only part of the Packet is sent out
> and the communication will break.
>

I think you are right, because that socket is set to non-blocking:

https://github.com/apache/zookeeper/blob/trunk/src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java#L267

So it looks like the sock.write call should be wrapped into something like:

  while (pbb.remaining() > 0) {
      sock.write(pbb);
  }

Do you mind filing a ticket for this? Thanks!


-rgs