[net][PATCH] TelnetInputStream.read sometimes hangs if reader thread is disabled

• Key: NET-3
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Unassigned
• Reporter: Rob Hasselbaum

Environment

Operating System: Windows XP
Platform: PC

Description

I'm trying to use TelnetClient with the reader thread disabled because I don't
want socket timeouts to fire during planned periods of inactivity (COM-1554).

But when the thread is disabled, I'm finding that TelnetInputStream.read
occassionally hangs when I try to read output from the server.
The problem appears to be the first while loop in __processChar, which looks
like this:

synchronized (__queue)
{
while (__bytesAvailable >= __queue.length - 1)
{
if(__threaded)
{
__queue.notify();
try

{ __queue.wait(); }

catch (InterruptedException e)

{ throw e; }

}
...
}

If you get into this loop and the threaded flag is false, you are stuck
forever. That's what's happening in my case. If I suspend the thread, I can
see that (_bytesAvailable) is 2048 and (_queue.length -
1) is also 2048, so it's an infinite loop.

I'm not sure what triggers this, but it seems to happen most often when there
is a pause in server output or a pause before I initiate the next read.

Comments

[Rob Hasselbaum]

Note that the code snippet in the description is missing the closing brace of
the "while" loop. It occurs right after the "if(__threaded)" block.

[Daniel Savarese]

I tried adding Bruno D'Avanzo to the cc list unsuccessfully as this appears to
be related to code he wrote and he may know better why it's written the way it
is. He may have gone inactive. I'll try to find his non-apache.org email
address. Unfortunately, I don't have enough spare cycles right now to
investigate this

[Siddique]

I too have experienced the same problem... any solutions for this?

[Rob Hasselbaum]

Does anyone have knowledge of this code?

I am willing to put time into a patch, but the documentation for the class is
VERY sparse. I'm afraid I'll just end up breaking it more.

[Rob Hasselbaum]

Created an attachment (id=17905)
Proposed patch for hanging read bug

[Rob Hasselbaum]

I believe what's happening is the buffer (queue) fills up and there is still
more data to be read from the socket. When this happens in threaded mode, the
reader thread waits in __processChar, giving the consumer thread a chance to
drain the buffer. That seems to work fine.

When threaded mode is disabled, however, we need to stop collecting data from
the socket and return some of what we have so far to the caller. In other words,
we shouldn't read any more from the socket or call __processChar unless there is
still room left in the queue to hold new data.

I've submitted a patch that I think fixes the problem. But since I'm unfamiliar
with this code (and this is my first Commons contribution), please take a close
look!

Basically, what I've done is add a check in read() to the while loop that was
adding data to the queue for non-threaded mode. If the queue is full, the loop
will terminate now. I also added a safety check to the code in __processChar
that will throw an IllegalStateException if it is called in non-threaded mode
and the queue is full. That should never happen.

All unit tests pass, and the new code prevented hangs in my ad hoc tests. I
tested it by submitting a command via Telnet to a host that causes it to respond
with a lot of data (e.g. "cat somebigtextfile.txt"). Then I put the client
thread to sleep for a few seconds before trying to read any of the response.
This consistently caused a hang with the old code, but not with the patch.

[Daniel Savarese]

Applied patch. Thanks Rob.

[Martin Oberhuber]

It looks like this patch has been applied for an "1.4.x" version of Commons Net,
but I cannot find such a released version anywhere.

How can I get a version of Commons Net that's compatible with 1.4.1 but has the patch?
Are there any plans for releasing 1.4.2 or 1.5?
FYI, at Eclipse DSDP-TM (http://www.eclipse.org/dsdp/tm) we're tracking this as
https://bugs.eclipse.org/bugs/show_bug.cgi?id=202758

[Elliott Rabe]

It also appears to be possible to run into this bug in threaded mode. It is possible for the close method to set the threaded state to false while the reader is still active. Under these circumstances the same infinite loop is invoked. I am also curious when the next build will be happening so I can see if the patch fixes this situation as well.

[Rory Winston]

You can try the prerelease at

http://people.apache.org/~rwinston/commons-net-2.0/

[Henri Yandell]

Reclosing so it gets marked as Fixed. Jira migration bug.