tail-f-systems / JNC

JNC (Java NETCONF Client) is the name of a Java library for communicating with NETCONF agents, and a plugin for pyang (http://code.google.com/p/pyang/) to generate Java classes from YANG models, to be used by the JNC library.
Other
77 stars 87 forks source link

close method on SSHSession.java #152

Open tsoihim opened 3 months ago

tsoihim commented 3 months ago

Hello. I have a question on SSHSession.java

I found that close() method on SSHSession doesn't close InputStream. Is there any reason?

It may cause thread hang on readData() method since no InputStream closed after rdr.read() called.

P.S. I appreciate your efforts in this project :)

martin-volf commented 3 months ago

The input stream used by SSHSession belongs to subsystem, an instance of Session.Subsystem; the method subystem.close() (which is called) takes care of closing its streams, as far as I can tell - so no reason to call close() on any of the streams.

What version are you using?

tsoihim commented 3 months ago

I'm using a fork from master branch (5 commits behind), but it shares same SSHSession code. The problem is, readData() stucks at rdr.read() when there is abnormal disconnection.

        @Override
        public <DataBufType> int readData(BaseReader<DataBufType> rdr, DataBufType buf,
                                          int offset, int length) throws IOException {
            watchdog.watch();
            try {
                int read = rdr.read(buf, offset, length);
                if (read == -1) {
                    trace("end of input (-1)");
                    throw new IOException("Session closed");
                } else {
                    ByteBuffer data = rdr.encode(buf, offset, read);
                    // FIXME: this is broken, readData may be called repeatedly
                    // on the same data due to buffering and mark/reset calls
                    for (final IOSubscriber sub: ioSubscribers) {
                        sub.inputRaw(data);
                    }
                }
                return read;
            } finally {
                watchdog.done();
            }
        }

Below are jstack result on hanging thread

"10.0.117.151" #86 daemon prio=5 os_prio=0 cpu=624.84ms elapsed=141.16s tid=0x00007f7258340850 nid=0x1f08a1 in Object.wait()  [0x00007f72d9ac4000]
   java.lang.Thread.State: WAITING (on object monitor)
    at java.lang.Object.wait(java.base@17.0.12/Native Method)
    - waiting on <0x000000071253bbe8> (a net.schmizz.sshj.common.Buffer$PlainBuffer)
    at java.lang.Object.wait(java.base@17.0.12/Object.java:338)
    at net.schmizz.sshj.connection.channel.ChannelInputStream.read(ChannelInputStream.java:106)
    - locked <0x000000071253bbe8> (a net.schmizz.sshj.common.Buffer$PlainBuffer)
    at java.io.BufferedInputStream.fill(java.base@17.0.12/BufferedInputStream.java:244)
    at java.io.BufferedInputStream.read1(java.base@17.0.12/BufferedInputStream.java:284)
    at java.io.BufferedInputStream.read(java.base@17.0.12/BufferedInputStream.java:343)
    - locked <0x0000000712554628> (a java.io.BufferedInputStream)
    at com.tailf.jnc.framing.ByteReader.read(BaseReader.java:39)
    at com.tailf.jnc.framing.ByteReader.read(BaseReader.java:25)
    at com.tailf.jnc.SSHSession$SessionDataReader.readData(SSHSession.java:137)
    at com.tailf.jnc.framing.NC1_1_Framer.readChunkHeader(Framer.java:144)
    at com.tailf.jnc.framing.NC1_1_Framer.parseFrame(Framer.java:164)
    at com.tailf.jnc.SSHSession.readOne(SSHSession.java:260)
    at com.tailf.jnc.NetconfSession.recvRpcReply(NetconfSession.java:1625)
    at com.tailf.jnc.NetconfSession.recvRpcReplyData(NetconfSession.java:1609)
    at com.tailf.jnc.NetconfSession.get(NetconfSession.java:644)
    at com.attoresearch.anchor.ubiquossdriver.UbiquossDriverImpl.sendGetRequest(UbiquossDriverImpl.java:149)
    at com.attoresearch.anchor.ubiquossdriver.services.state.SwitchStatusServiceImpl.getProtocol(SwitchStatusServiceImpl.java:94)
    at com.attoresearch.anchor.ubiquossdriver.services.state.SwitchStatusServiceImpl.getProtocolStatus(SwitchStatusServiceImpl.java:67)
    at com.attoresearch.nouvelle.switchframe.SwitchHandlerWorker.runHealthCheck(SwitchHandlerWorker.java:103)
    at com.attoresearch.nouvelle.switchframe.SwitchHandlerWorker.run(SwitchHandlerWorker.java:162)
    at java.lang.Thread.run(java.base@17.0.12/Thread.java:840)
martin-volf commented 2 months ago

There are definitely bugs (more than one, I think) in how readTimeout is handled, but I'm not sure if this is what you are complaining about. With the default readTimeout, the session would wait for incoming data pretty much indefinitely, or until the connection is terminated; you write that you have "abnormal disconnection" - what does that mean? If there is a disconnection and the client knows about it (i.e. the operating system knows about it), everything should just work in the sense that the session terminates; otherwise, if the client does not know about the disconnection, it just waits, and there is hardly anything else that it can do (except for timing out on a readTimeout if it has been set - but that does not work currently, as I wrote).