tomaswolf commented on a change in pull request #163:
URL: https://github.com/apache/mina-sshd/pull/163#discussion_r484566434
##########
File path:
sshd-core/src/main/java/org/apache/sshd/common/channel/BufferedIoOutputStream.java
##########
@@ -56,7 +56,12 @@ public IoWriteFuture writePacket(Buffer buffer) throws
IOException {
}
IoWriteFutureImpl future = new IoWriteFutureImpl(getId(), buffer);
- writes.add(future);
+ try {
+ writes.put(future);
+ } catch (InterruptedException e) {
+ Thread.interrupted();
+ throw new IOException(e);
+ }
Review comment:
I know about the semantics of `put`. Yes, it'll block indefinitely until
there is space again - which will be when the slow consumer has caught up. It
doesn't make any sense to let the producer continue any earlier. But maybe
there's a better place to implement this throttle than in this
`BufferedIoOutputStream`.
Your suggestion is a larger refactoring that I cannot do. Using `offer` with
timeout one also would have to handle in the caller the case that the internal
queue was full, and wait there, and there'd need to be a way to configure the
queue size. Moreover the queue size and timeout needed would depend on the
relative speeds of producer and consumer, which are unknown in general. I
wouldn't know how to configure such a thing such that it worked for all cases.
Just failing the transfer when the queue is ever full is IMO not an option; the
whole mechanism should probably still work (albeit slowly) with a very fast
producer and a very slow consumer.
The problem is precisely that current code doesn't place any restrictions,
and thus a fast producer can overrun a slow consumer so much that the server
gets an OOME.
But in any case SSHD-1070 and SSHD-1069 should probably be resolved together.
BTW, `Thread.interrupted()`is wrong; should have been
`Thread.currentThread().interrupt()`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
