Hello Atsuhiko, yes this patch looks like a good solution that should fix the problem safely.
But why do you synchronize on Session.class and not on this ? It seems that this would unnecessarily influence other Sessions that have absolutely nothing to do with this one. Also, synchronizing on this seems to have the added value that synchronized methods "setConfig" and "_write" can not run concurrently. Cheers, -- Martin Oberhuber, Senior Member of Technical Staff, Wind River Target Management Project Lead, DSDP PMC Member http://www.eclipse.org/dsdp/tm > -----Original Message----- > From: Atsuhiko Yamanaka [mailto:[EMAIL PROTECTED] > Sent: Monday, July 14, 2008 9:13 AM > To: Oberhuber, Martin > Cc: [email protected] > Subject: Re: [JSch-users] JSch-0.1.37: NPE in > Session.disconnect() when calledbefore authenticated > > Hi, > > +-From: "Oberhuber, Martin" <[EMAIL PROTECTED]> -- > |_Date: Fri, 11 Jul 2008 17:14:36 +0200 _______________________ > | > |In JSch-0.1.37, Session.connect(int) sets > |isConnected=true in line 208, but the > |connectThread is only created later after > |Authentication is done. > ... > |I'm not sure how to best address this, but one > |option might be that disconnect() does this: > |if(connectThread==null) { > | //Not yet authenticated: Signal cancellation > | synchronized(this) { > | isConnected = false; > | } > |} > > In that change, there is a possibility that > 'connectThread' is assigned before entering synchronized clause. > And then, 'this' (an instance of Session) has been already > used to send data, so 'disconnect' may be blocked in some case. > > How about the following patch? > > --- jsch-0.1.39/src/com/jcraft/jsch/Session.java Thu Jun > 12 07:06:49 2008 > +++ jsch-0.1.40/src/com/jcraft/jsch/Session.java Mon Jul > 14 05:05:36 2008 > @@ -456,12 +456,21 @@ > } > > isAuthed=true; > - connectThread=new Thread(this); > - connectThread.setName("Connect thread "+host+" session"); > - if(daemon_thread){ > - connectThread.setDaemon(daemon_thread); > + > + synchronized(Session.class){ > + if(isConnected){ > + connectThread=new Thread(this); > + connectThread.setName("Connect thread "+host+" session"); > + if(daemon_thread){ > + connectThread.setDaemon(daemon_thread); > + } > + connectThread.start(); > + } > + else{ > + // The session has been already down and > + // we don't have to start new thread. > + } > } > - connectThread.start(); > } > catch(Exception e) { > in_kex=false; > @@ -1487,10 +1496,12 @@ > PortWatcher.delPort(this); > ChannelForwardedTCPIP.delPort(this); > > - synchronized(connectThread){ > - Thread.yield(); > - connectThread.interrupt(); > - connectThread=null; > + synchronized(Session.class){ > + if(connectThread!=null){ > + Thread.yield(); > + connectThread.interrupt(); > + connectThread=null; > + } > } > thread=null; > try{ > ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ JSch-users mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/jsch-users
