FTPClient deals badly with adverse network conditions

• Key: NET-30
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Unassigned
• Reporter: Alain Knaff

Environment

Operating System: Linux
Platform: PC

Description

We are attempting to use the FTPClient included in commons-net 1.2.2 to write
an application which has to deal with various outages:

• connection closed by server, without 421 warning
• connection frozen
• server temporarily unreachable
• ...

However, it appears that in most of these cases, FTPClient becomes easily
confused:

• If the connection is closed without warning, FTPClient doesn't notice at
  first: ftp.isConnected() still returns true. However, when attempting to use
  such a closed connection, we do get the correct exception
  (FTPConnectionClosedException), at least most of the time (occasionnally we do
  get various SocketExceptions instead)
• If the ftp server doesn't respond in time, FTPClient hangs forever, even
  after the ftp server responds eventually
• If setSoTimeout is set, and if the server doesn't respond, the following
  behaviour happens:
  • if the server becomes responsive again before the timeout happens, the
    client hangs until the timeout then receives SocketTimeoutException exception.
    We would prefer if the client just continued normally (as the server did
    respond before timeout)
  • if on the other hand the server does not become responsive when the timeout
    happens, the client won't get any exception when timeout expires. Instead it
    will just hang. As soon as the server gets responsive again (i.e. after the
    timeout had expired), the client immediately get the exception. We would prefer
    if the client got the exception at the timeout, rather than long afterwards.

Comments

[Alain Knaff]

Some small corrections:

• Problem number 2 and 3 ("hang" if ftp server responds slowly, badly timed
  SocketTimeout exceptions) was actually due to an artifact of vsftpd that we
  used for testing. Please disregard these item, sorry for the confusion.

However the following point still stands:
1. ftp.isConnected() does not notice that connection has been closed

Additionnally, there are 2 new points:

4. The timeout set by setSoTimeout misfires if the client hasn't submitted a
request for a while. Probably some internal thread is waiting permanently for
server "responses" even if the client didn't send the server anything to
respond to. IMHO, timeout should only be triggered if an expected response
doesn't come.
5. There apparently is no way of setting a timeout for the initial connection
request. Even setDefaultTimeout doesn't work for completely that purpose, as
can be easily checked by attempting to connect to an unreachable IP address.

[Alain Knaff]

One additional note:

Point #4 (misfiring of soTimeout if client is idle) can be avoided by calling
ftpclient.setReaderThread(false). Might be appropriate though to mention this
in the javadoc, in the description of setSoTimeout() and setDefaultTimeout()

[Daniel Savarese]

Regarding 1, isConnected is not supposed to notice if a connection has
terminated abnormally. It is intended that one catch an exception in
such a case.

4. The socket timeout is exactly that. If there's no activity on a socket
for a given amount of time, it times out. The timeout happens in the
TCP stack and java sets both SO_SNDTIMEO and SO_RCVTIMEO.

5. setDefaultTimeout is merely the socket timeout to set after a connection
is established. There is no support for connection timeouts prior to J2SE 1.4,
but Commons Net 1.2 is supposed to be J2SE 1.2 compatbile. If you want to
set a connection timeout, you can create a socket factory as described in:
http://www.mail-archive.com/[e-mail]/msg07716.html

[Daniel Savarese]

Re: setReaderThread
I didn't know we had such a method. It looks like it was added here:
http://cvs.apache.org/viewcvs.cgi/jakarta-commons/net/src/java/org/apache/commons/net/telnet/TelnetClient.java?rev=1.9&view=markup

Other than adding documentation to FTPClient about this, it sounds like
like there isn't any other action to take on this report???

[Alain Knaff]

Indeed, if point #1 (ftp.isConnected()) is the intended behavior, and if #5
(connect timeout) cannot be addressed in JDK 1.2, then the only action left is
indeed #4, i.e. a documentation fix.

Or maybe, it could be fixed in the code by testing __readIsWaiting in
TelnetInputStream.run(): if __readIsWaiting is not set when the
InterruptedIOException occurs, just loop around without storing it into
_ioException. That way it would work both in __threaded and non-_threaded
mode. The difficulty however for such a fix would be the border cases, such as
for example the situation where a user-level read() happens when the timeout
is already half expired, leaving only half to go. Maybe something could be
done by recording the start times of run's read() and of the user-level
read(), and dynamically adjusting the soTimeout?

[Daniel Savarese]

The root of the problem is the nasty telnet code. As I indicated on
commons-dev in response to a related issue:
I think the long term solution
should be to branch, leaving the 1.2 series as J2SE 1.2 and 1.3 compatible,
and reimplement telnet using selectable I/O in a single thread on the
head branch; or start a new 2.0 branch either as a branch or in the
sandbox as commons-net2, since everything would have to be overhauled
once we start using java.nio.
Basically, I think we're seeing enough user demand that we should contemplate
the move to J2SE 1.4, even if we have to maintain two separate branches
(although I prefer the commons-net2 approach). I think we've taken
incremental patching as far as we can.

On a separate note, the whole isConnected behavior issue needs to be
sorted out in a 2.0 version. There has been enough of an expectation
from users as far as FTPClient is concerned for it to report the
current state of the underlying socket. Previous to J2SE 1.4, it
was not possible to to test that state. But Socket.isConnected
and SocketChannel.isConnected were added in J2SE 1.4, so that is
what a future SocketClient.isConnected should return. Keep in mind
that you should be able to subclass FTPClient and override isConnected
if you're using J2SE 1.4 to return Socket.isConnected (I think).

[Rory Winston]

I inserted a a short note and a link in the Javadoc to this thread. Will close
for now.

[Henri Yandell]

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