mwiede / jsch

fork of the popular jsch library
Other
761 stars 144 forks source link

Inappropriate logging "Caught an exception, leaving main loop due to Socket closed" on disconnect #99

Open vbedrosova opened 2 years ago

vbedrosova commented 2 years ago

If I provide a logger accepting severity >= INFO via com.jcraft.jsch.JSch#setLogger it always reports the mentioned "Caught an exception, leaving main loop due to Socket closed" on disconnect" message after "Disconnecting from port ". It looks confusing, because happens even in regular scenarios. Would be great if you could resolved this, thanks!

mwiede commented 2 years ago

Hi @vbedrosova can you elaborate, what a regular scenario is for you? I tried to reproduce an simple Exec-example, but I did not come into exception block in https://github.com/mwiede/jsch/blob/master/src/main/java/com/jcraft/jsch/Session.java#L2018 From my point of view the logging level should be switched to ERROR thow.

curtisjenkins commented 2 years ago

After each "Disconnecting from ..... port 22", I see "Caught an exception, leaving main loop due to Socket closed". It's an INFO level log message.

zigzag75 commented 2 years ago

Hi @mwiede we are also facing this issue. Our scenario is quite simple :

The mix of INFO level log and worrying message is not comfortable. Could you have a look please ? Many thanks

takkiraz commented 1 year ago

Hi @mwiede any news about this? I've the same scenario as @zigzag75 mentioned.

canchanchara commented 1 year ago

I have the same problem as @zigzag75 . I do also ask for any news about this

precoder commented 1 year ago

We are also getting the same issue, however our Authentication works in an irregular way. Sometimes we can login into Server sometimes we can not, first assuming this is a Problem on Server side..

amit1sharma commented 1 year ago

Yes, same error. With me, every alternate call to upload a file to SFTP is failing with the above-mentioned message and the very next call(after failure) to upload the same document is creating a new connection and uploads the file successfully.

karand1985 commented 1 year ago

Yes, same error occurs on every disconnect of SFTP connection on port 22. the version we are using is 0.1.62 in our application

Regards, Karan

Qveshn commented 7 months ago

@mwiede Hi IMHO: This is due to the incorrect implementation of the Session thread stopping in Session.disconnect() This method does not wait main "while-loop" exiting in Session.run(). It closes all resources (including socket), but thread is still working. I think you should wait until "while-loop" exiting and thread stopping after "isConnected = false;" or "thread = null;". And only then close all resources (IO, Socket, etc.)

May be this exception will help (after calling disconnect and then wait a little):

java.net.SocketException: Socket closed
    at java.net.SocketInputStream.socketRead0(Native Method)
    at java.net.SocketInputStream.socketRead(SocketInputStream.java:116)
    at java.net.SocketInputStream.read(SocketInputStream.java:171)
    at java.net.SocketInputStream.read(SocketInputStream.java:141)
    at com.jcraft.jsch.IO.getByte(IO.java:95)
    at com.jcraft.jsch.Session.read(Session.java:1182)
    at com.jcraft.jsch.Session.run(Session.java:1783)
    at java.lang.Thread.run(Thread.java:748)
bodhi-one commented 1 month ago

This issue is open for nearly 4 years now. We are also seeing issues here. Is jsch still being maintained?

norrisjeremy commented 1 month ago

Hi @bodhi-one,

Yes, this JSch fork is still maintained. And no, we do not have currently have any plans to implement a change, as this is not actually an issue. The log message is harmless, so I would suggest you simply ignore it and carry on.

Thanks, Jeremy

bodhi-one commented 1 month ago

Hi @bodhi-one,

Yes, this JSch fork is still maintained. And no, we do not have currently have any plans to implement a change, as this is not actually an issue. The log message is harmless, so I would suggest you simply ignore it and carry on.

Thanks, Jeremy

Hi Jeremy, so do you think this suggestion is incorrect ? :

@mwiede Hi IMHO: This is due to the incorrect implementation of the Session thread stopping in Session.disconnect() This method does not wait main "while-loop" exiting in Session.run(). It closes all resources (including socket), but thread is still working. I think you should wait until "while-loop" exiting and thread stopping after "isConnected = false;" or "thread = null;". And only then close all resources (IO, Socket, etc.)

norrisjeremy commented 1 month ago

Hi @bodhi-one,

Nobody has demonstrated an actual bug as of yet: all anyone has noted is that they get what seems to be a harmless log message. I am also extremely hesitant to make any changes with this code, as it is very old and extremely fragile, and I don't want to accidentally introduce any regressions.

Thanks, Jeremy

bodhi-one commented 1 month ago

@mwiede Hi IMHO: This is due to the incorrect implementation of the Session thread stopping in Session.disconnect() This method does not wait main "while-loop" exiting in Session.run(). It closes all resources (including socket), but thread is still working. I think you should wait until "while-loop" exiting and thread stopping after "isConnected = false;" or "thread = null;". And only then close all resources (IO, Socket, etc.)

I've looked into the Session class a bit. There are lots of commented old code and places where exceptions could occur and are never logged, just eaten and disconnected. This kind of stuff should at least get fixed. Does not impact the overall logic, just needs better logging. You should never eat an exception without logging like this.

try { channel.write(foo, start[0], length[0]); } catch (Exception e) { // System.err.println(e); try { channel.disconnect(); } catch (Exception ee) { } break; }

norrisjeremy commented 1 month ago

Hi @bodhi-one,

You are welcome to submit PRs that we will review if you feel it is important. Please do remember however:

  1. JSch was originally authored by someone besides @mwiede and myself who has long since disappeared from the Internet: we have simply picked up from where they left off & continued to maintain the code base, trying to keep it up-to-date with changing SSH standards & crypto algorithms.
  2. This codebase is old and can be extremely fragile, so we tend to be very conservative with making functional changes unless it fixes a material bug that a user is experiencing or adds a useful new feature, for fear of introducing a regression; so please remember that fact if you do decide to submit PRs.

Thanks, Jeremy