You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Tapan Karecha <ta...@india.hp.com> on 2002/09/19 13:35:43 UTC
[patch] NetComponents - TelnetInputStream.java threading issue
While using NetComponents library on HP-UX 11i platform, even after calling
close() on FTPClient object, the TelnetInputStream thread does not always
terminate; and blocks on wait() for ever. The patch below addresses this
issue. I've tested this fix on Windows and HP-UX and found it OK. Can
someone apply this patch? Thanks!!
--- TelnetInputStream.java.orig Thu Sep 19 15:48:50 2002
+++ TelnetInputStream.java Thu Sep 19 15:48:38 2002
@@ -39,7 +39,7 @@
_STATE_WONT = 3, _STATE_DO = 4, _STATE_DONT = 5, _STATE_SB = 6,
_STATE_SE = 7, _STATE_CR = 8;
- private boolean __hasReachedEOF, __isClosed;
+ private volatile boolean __hasReachedEOF, __isClosed;
private boolean __readIsWaiting;
private int __receiveState, __queueHead, __queueTail, __bytesAvailable;
private int[] __queue;
@@ -300,12 +300,12 @@
// interrupt a system read() from the interrupt() method.
super.close();
- synchronized(__queue) {
__hasReachedEOF = true;
+ __isClosed = true;
if(__thread.isAlive()) {
- __isClosed = true;
__thread.interrupt();
}
+ synchronized(__queue) {
__queue.notifyAll();
}
/*
@@ -332,10 +332,10 @@
} catch(InterruptedIOException e) {
synchronized(__queue) {
__ioException = e;
- __queue.notify();
+ __queue.notifyAll();
try {
//System.out.println("THREAD WAIT B");
- __queue.wait();
+ __queue.wait(100);
//System.out.println("THREAD END WAIT B");
} catch(InterruptedException interrupted) {
if(__isClosed)
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: [patch] NetComponents - TelnetInputStream.java threading issue
Posted by Tapan Karecha <ta...@india.hp.com>.
Thanks Daniel for applying the patch. Yes, I did not do the diff against the
latest code. My mistake.
I could not think of a way to unit test this fix. In fact, it took me a
while before I could figure out that the threads were not terminating as
they should. The application that uses FTPClient used to run out of memory
once or twice a week. We saw the hanging threads only when we ran a
profiler. With this fix, we stopped seeing the dangling threads on running
the profiler.
Earlier on the yahoo mailing list, you, Jeff and I discussed the restart
issue with FTPClient. I have a patch to fix this problem (the diff is done
on the latest code this time), for which I shall send a patch in a separate
mail.
- Tapan.
> -----Original Message-----
> From: Daniel F. Savarese [mailto:dfs@savarese.org]
> Sent: Thursday, September 19, 2002 10:26 PM
> To: commons-dev@jakarta.apache.org
> Subject: Re: [patch] NetComponents - TelnetInputStream.java threading
> issue
>
>
>
> "Tapan Karecha" <ta...@india.hp.com> wrote:
> >While using NetComponents library on HP-UX 11i platform,
> even after calling
> >close() on FTPClient object, the TelnetInputStream thread
> does not always
> >terminate; and blocks on wait() for ever. The patch below
> addresses this
> >issue. I've tested this fix on Windows and HP-UX and found it OK. Can
> >someone apply this patch? Thanks!!
>
> The patch doesn't seem to be generated against what's in
> jakarta-commons-sandbox/net so I can't apply it directly.
> In the future, the telnet package should probably disappear
> (unless anyone wants to make it more useful) and the telnet
> conversation code needed for FTP handled in a single thread, but
> that's another story. We're stuck with my imperfect original
> implementation for the time being.
>
> In the patch, making __hasReachedEOF and __isClosed volatile
> doesn't do
> anything. Those are not static variables. Changing the notify() to
> notifyAll() in run() still leaves you at the mercy of the thread
> scheduler. The real key is probably the __queue.wait(100). Anyway,
> my point is that I don't think all of your changes are necessary to
> address the problem. Nonetheless, I preserved almost all of
> your changes
> and inserted the code by hand. Ultimately, there's probably
> a problem with
> setting the thread priority in _start(), which in theory you shouldn't
> have to do, but it was the only way to get this approach to work with
> green threads and other user-space thread implementations such as
> the one in the original MacOS 8 JDK.
>
> Please do a checkout of the jakarta-commons-sandbox/net code and
> try that out in your environment. Also, it would be appreciated
> if you submitted a unit test that would expose the problem you
> ran into so we can include that as a regression test. As Jeff
> indicated, we don't want to make anything other than very small
> maintenance changes to the code right now, so I think we should
> avoid making any fixes that can't be verfied with a corresponding
> unit test.
>
> daniel
>
>
>
> --
> To unsubscribe, e-mail:
> <ma...@jakarta.apache.org>
> For additional commands, e-mail:
> <ma...@jakarta.apache.org>
>
>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [patch] NetComponents - TelnetInputStream.java threading issue
Posted by "Daniel F. Savarese" <df...@savarese.org>.
I wrote:
>In the patch, making __hasReachedEOF and __isClosed volatile doesn't do
>anything. Those are not static variables. Changing the notify() to
Er, what I meant to say is that assignments to those variables are already
protected by critical sections. Your patch removed one of those protections,
so the volatile would have been appropriate. I left in the protection.
daniel
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [patch] NetComponents - TelnetInputStream.java threading issue
Posted by "Daniel F. Savarese" <df...@savarese.org>.
"Tapan Karecha" <ta...@india.hp.com> wrote:
>While using NetComponents library on HP-UX 11i platform, even after calling
>close() on FTPClient object, the TelnetInputStream thread does not always
>terminate; and blocks on wait() for ever. The patch below addresses this
>issue. I've tested this fix on Windows and HP-UX and found it OK. Can
>someone apply this patch? Thanks!!
The patch doesn't seem to be generated against what's in
jakarta-commons-sandbox/net so I can't apply it directly.
In the future, the telnet package should probably disappear
(unless anyone wants to make it more useful) and the telnet
conversation code needed for FTP handled in a single thread, but
that's another story. We're stuck with my imperfect original
implementation for the time being.
In the patch, making __hasReachedEOF and __isClosed volatile doesn't do
anything. Those are not static variables. Changing the notify() to
notifyAll() in run() still leaves you at the mercy of the thread
scheduler. The real key is probably the __queue.wait(100). Anyway,
my point is that I don't think all of your changes are necessary to
address the problem. Nonetheless, I preserved almost all of your changes
and inserted the code by hand. Ultimately, there's probably a problem with
setting the thread priority in _start(), which in theory you shouldn't
have to do, but it was the only way to get this approach to work with
green threads and other user-space thread implementations such as
the one in the original MacOS 8 JDK.
Please do a checkout of the jakarta-commons-sandbox/net code and
try that out in your environment. Also, it would be appreciated
if you submitted a unit test that would expose the problem you
ran into so we can include that as a regression test. As Jeff
indicated, we don't want to make anything other than very small
maintenance changes to the code right now, so I think we should
avoid making any fixes that can't be verfied with a corresponding
unit test.
daniel
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>