Client wrongly detects disconnection from server

Bug #708046 reported by Omry Yadan on 2011-01-26
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Gearman Java
Undecided
Unassigned

Bug Description

In GearmanNIOJobServerConnection.read() :

            int bytesRead = serverConnection.read(bytesReceived);

            if (bytesRead >= 0) {
                ...
            } else {
            if (isOpen())
                close();
                //TODO do something smarter here
                throw new IOException("Connection to job server severed");
            }

I believe this is incorrect.
the documentation of ReadableByteChannel.read() states that it may return 0 bytes if it has nothing to read.

in other words, the client disconnects the connection just because its reading faster than the server sends.
this is unlikely to happen if you are working locally, but can (and does) happen otherwise.
Based on the doc, if the channel gets closed - a ClosedChannelException will be thrown.

* http://download.oracle.com/javase/1.5.0/docs/api/java/nio/channels/ReadableByteChannel.html

Omry Yadan (omry) wrote :

I pushed a fix to my reconnect-bug branch, so far it seems to work well.

Omry Yadan (omry) wrote :

for reference, second if is the fix:

            int bytesRead = serverConnection.read(bytesReceived);
            if (bytesRead >= 0) {
                  ...
            }
            else
            if (bytesRead == -1){
         if (isOpen())
             close();
                //TODO do something smarter here
                throw new IOException("Connection to job server severed");
            }

Eric Lambert (elambert) wrote :

Omry:

I am little confused about what you think is the problem here. Are you saying that if serverConnection.read(bytesReceived) returns 0, we are improperly closing the connection? That does not match with how I read the code. Based on how I read the original code (see snippet below), we should only execute the "else" clause (which performs the close and throws the exception) if bytesRead is *less than* 0 (essentially if it is -1). A bytesRead value of 0 should result in the "if" clause being executed because the test for the "if" clause is that bytesRead is *greater-than-or-equal*.

Am I mis-understanding something here?

int bytesRead = serverConnection.read(bytesReceived);
if (bytesRead >= 0) {
  ...
} else {
  if (isOpen())
    close();
    //TODO do something smarter here
    throw new IOException("Connection to job server severed");
}

Omry Yadan (omry) wrote :

you are absolutely right and I am totally confused now because this change did improve the situation.
I`ll take a closer look and continue working on this, think you can close this bug.

Eric Lambert (elambert) wrote :

Marking as invalid for the time being as the code does behave correctly when a read returns a 0 value. Although there is still the underlying issue of the disconnect

Changed in gearman-java:
status: New → Invalid
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers