Mailing-List: contact commons-user-help@jakarta.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Jakarta Commons Users List" <commons-user@jakarta.apache.org>
Message-Id: <200401090623.i096NlKF011983@gandalf.savarese.org>
To: "Jakarta Commons Users List" <commons-user@jakarta.apache.org>
Subject: Re: [net] random NPEs on read 
In-reply-to: Your message of "Thu, 08 Jan 2004 09:41:51 PST."
             <378277A8F8AD4049AD8BC80D1F2B953930AD6D@sawmailsf1.sawyermedia.local>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Date: Fri, 09 Jan 2004 01:23:47 -0500
From: "Daniel F. Savarese" <dfs@savarese.org>


In message <378277A8F8AD4049AD8BC80D1F2B953930AD6D@sawmailsf1.sawyermedia.local
>, "Jeff Barrett" writes:
>I'm using commons-net to ftp a bunch of files around every night as a =
>batch process.  It probably copies just under 1g.  Over the course of =
>that process I'll  often get 2 or 3 thrown exceptions like so:

That's all happening in BufferedInputStream.  Looking at the code
for BufferedInputStream in J2SE 1.4.2, it looks like the
NullPointerException exception is happening at a call of in.available().
in is set to null in close().  So it seems there may be a situation
whereby a read by TelnetInputStream.__read is being performed after
a close.  Looking at the code, I'm pretty sure there's a problem.
TelnetInputStream calls super.close() before entering a
synchronized block.  There's a race condition (unrelated to the
synchronized block, which protects something else) that could
allow the reader thread to start a read after BufferedInputStream.close()
is called.  The reason it hardly ever happens is that
BufferedInputStream calls an ensureOpen() method at the
beginning of read() that checks to make sure in is not null.
However, between that call and the call to in.available(),
the value may become null because BufferdInputStream.close()
is not a synchronized method, unlike BufferedInputStream.read().

Now, there's a comment in the code before the close
call that I'm guilty of that says:
        // Completely disregard the fact thread may still be running.
        // We can't afford to block on this close by waiting for
        // thread to terminate because few if any JVM's will actually
        // interrupt a system read() from the interrupt() method.
This means the close() is being used to force the read() to return -1
or throw an exception.  If we synchronize the call to close
(synchronizing on the input stream), we may get deadlock.  Mind you,
this is circa JDK 1.1.x.

The good news is that the NullPointerException is harmless
because it's happening when the stream is being closed and the thread
is terminated.  Also, it's the FTP control stream, not the data stream
that's being affected, so there will be no loss of data.  The bad
news is that I'm not immediately sure how to fix it in light of the
code comment.  This is a great example of why I'd like to get rid of
the threads and move to java.nio for version 2.0 now that we can
use the Java equivalent of select().

I can't come up with any ideas that don't run the risk of
deadlock on some JVM.  I'm afraid the right fix may be to
simply catch any RuntimeExceptions thrown by read and take
the same action as a close().  We probably should have been
doing that anyway to gracefully handle unexpected situations.
As an interim fix, I applied that change.  You have to do
a CVS checkout of the latest source and compile to get the
fix.

daniel


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org