[
https://issues.apache.org/jira/browse/SSHD-1125?focusedWorklogId=558589&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-558589
]
ASF GitHub Bot logged work on SSHD-1125:
----------------------------------------
Author: ASF GitHub Bot
Created on: 26/Feb/21 15:29
Start Date: 26/Feb/21 15:29
Worklog Time Spent: 10m
Work Description: lgoldstein commented on a change in pull request #181:
URL: https://github.com/apache/mina-sshd/pull/181#discussion_r583720123
##########
File path:
sshd-core/src/main/java/org/apache/sshd/common/channel/BufferedIoOutputStream.java
##########
@@ -52,60 +78,164 @@ public Object getId() {
@Override
public IoWriteFuture writeBuffer(Buffer buffer) throws IOException {
if (isClosing()) {
- throw new EOFException("Closed - state=" + state);
+ throw new EOFException("Closed/ing - state=" + state);
}
+ waitForAvailableWriteSpace(buffer.available());
+
IoWriteFutureImpl future = new IoWriteFutureImpl(getId(), buffer);
writes.add(future);
startWriting();
return future;
}
+ protected void waitForAvailableWriteSpace(int requiredSize) throws
IOException {
+ long expireTime = System.currentTimeMillis() +
maxWaitForPendingWrites.toMillis();
+ synchronized (pendingBytesCount) {
+ for (int count = pendingBytesCount.get();
+ /*
+ * The (count > 0) condition is put in place to allow a
single pending
+ * write to exceed the maxPendingBytesCount as long as there
are no
+ * other pending writes.
+ */
+ (count > 0)
+ // Not already over the limit or about to be over it
+ && ((count >= maxPendingBytesCount) || ((count +
requiredSize) > maxPendingBytesCount))
+ // No pending exception signaled
+ && (pendingException.get() == null);
+ count = pendingBytesCount.get()) {
+ long remTime = expireTime - System.currentTimeMillis();
+ if (remTime <= 0L) {
+ pendingException.compareAndSet(null,
+ new SshChannelBufferedOutputException(
+ channelId,
+ "Max. pending write timeout expired after
" + writtenBytesCount + " bytes"));
+ throw pendingException.get();
+ }
+
+ try {
+ pendingBytesCount.wait(remTime);
+ } catch (InterruptedException e) {
+ pendingException.compareAndSet(null,
+ new SshChannelBufferedOutputException(
+ channelId,
+ "Waiting for pending writes interrupted
after " + writtenBytesCount + " bytes"));
+ throw pendingException.get();
Review comment:
I have always wondered about that - however there are other locations in
our code that catch this exception and there are exactly 2 locations where this
convention is maintained - both of them written by a contributor. So while I
may (or not) agree with you it is not part of our current coding convention, so
I stuck with what we did in the past. If you think we should review this
decision, let's do it through a separate issue and address all such locations
in the code.
----------------------------------------------------------------
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]
Issue Time Tracking
-------------------
Worklog Id: (was: 558589)
Time Spent: 50m (was: 40m)
> Provide a boundary on BufferedIoOutputStream writing to avoid memory overflow
> -----------------------------------------------------------------------------
>
> Key: SSHD-1125
> URL: https://issues.apache.org/jira/browse/SSHD-1125
> Project: MINA SSHD
> Issue Type: Bug
> Reporter: Lyor Goldstein
> Assignee: Lyor Goldstein
> Priority: Major
> Labels: memory
> Time Spent: 50m
> Remaining Estimate: 0h
>
> Use an upper bound to the data pending in the {{BufferedIoOutputStream}}. The
> max data could be set to the max window size. Blocking until there is enough
> room should allow the client to read the data.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
