adubrouski opened a new issue, #937:
URL: https://github.com/apache/mina-sshd/issues/937
### Version
3.0.0-M5, 3.0.0-M7, dev_3.0 HEAD
### Bug description
Since c3ba88c823 ("Introduce a KexFilter"), `AbstractChannel.init()`
registers a `KexListener` on the session for every channel:
```java
//
sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
(dev_3.0, line 401)
((AbstractSession) session).addKexListener(kexStarted -> {
...
});
```
Nothing removes that listener when the channel closes, and
`AbstractSession.removeKexListener()` could not remove it anyway, because it
adds the listener a second time:
```java
//
sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
(dev_3.0, line 300)
public void removeKexListener(KexListener listener) {
sshTransport.addKexListener(listener); // should be removeKexListener
}
```
The lambda captures the channel, so `KexFilter.listeners` (a
`CopyOnWriteArrayList`) keeps every channel the session has ever opened
reachable, together with everything the channel references: the
`ChannelOutputStream`, the local and remote windows, and the `OutputStream`s
passed to `setOut()`/`setErr()`. The memory is released only when the session
itself is closed.
This does not matter for a session that runs one command and is closed. It
does matter for a client that keeps one session open and runs many short exec
channels on it, which is the usual way to avoid paying for key exchange and
authentication on every command.
To reproduce:
1. Connect a `ClientSession` and keep it open.
2. Run a short command in a loop, closing each channel:
```java
for (int i = 0; i < 100_000; i++) {
ByteArrayOutputStream out = new ByteArrayOutputStream();
try (ChannelExec channel = session.createExecChannel("echo hello")) {
channel.setOut(out);
channel.open().verify(10_000);
channel.waitFor(EnumSet.of(ClientChannelEvent.CLOSED), 10_000);
}
}
```
3. Take a heap histogram: the number of `ChannelExec` instances equals the
number of channels opened, and all of them are retained by
`KexFilter.listeners`.
### Actual behavior
Closed channels are never garbage-collected while their session is open. In
our service (a few dozen long-lived sessions, each running a few commands per
second), the 24 GiB heap filled up in about 7 hours and the JVM ran out of
memory.
At that point a live heap dump showed:
- 65 `ClientSessionImpl` instances retaining 17.98 GB, which is 97% of the
heap;
- 186,151 `ChannelExec` instances, all closed, together with 186,151
`CloseableLocalWindow`, `CloseableRemoteWindow`, `ChannelOutputStream`,
`PtyChannelConfiguration` and so on;
- 17.5 GB in `byte[]`, mostly the `ByteArrayOutputStream`s passed to
`setOut()`/`setErr()`;
- a single session holding 2,842 closed channels.
The path to the GC root for one of the closed channels (Eclipse MAT,
`path2gc`):
```
ChannelExec
arg$1 AbstractChannel$$Lambda <- the KexListener from
AbstractChannel.init()
[93] Object[2842]
array CopyOnWriteArrayList
listeners KexFilter
kexFilter SshTransportFilter
sshTransport ClientSessionImpl
AbstractFactoryManager$1.sessions <-
sshd-SshClient-timer-thread
```
### Expected behavior
A closed channel is unreachable from its session and can be
garbage-collected, as in 2.x. Its `KexListener` is removed when the channel
closes, and `AbstractSession.removeKexListener()` removes the listener.
Suggested fix:
1. In `AbstractSession.removeKexListener()`, call
`sshTransport.removeKexListener(listener)`.
2. In `AbstractChannel`, keep the listener registered in `init()` in a
field, and remove it when the channel closes, for example in the `run(...)`
step of `getInnerCloseable()` next to `service.unregisterChannel(this)`.
### Relevant log output
```Shell
Peer: OpenSSH. Client: `SshClient` with the Netty I/O factory (sshd-netty),
Java 25.
The 2.x line is not affected: it has no `KexFilter`.
Our workaround: close each pooled session after a fixed number of commands,
so it never accumulates more than a bounded number of channels.
```
### Other information
_No response_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
