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:[EMAIL PROTECTED]] > Sent: Thursday, September 19, 2002 10:26 PM > To: [EMAIL PROTECTED] > Subject: Re: [patch] NetComponents - TelnetInputStream.java threading > issue > > > > "Tapan Karecha" <[EMAIL PROTECTED]> 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: > <mailto:[EMAIL PROTECTED]> > For additional commands, e-mail: > <mailto:[EMAIL PROTECTED]> > > -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
