This is an automated email from the ASF dual-hosted git repository. lgoldstein pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit 4fc3e04743987e807087c555187ae1a5d6a1410c Author: Lyor Goldstein <[email protected]> AuthorDate: Thu Aug 6 18:05:37 2020 +0300 [SSHD-1050] Store only the 1st error that occurs before first authentication --- .../sshd/client/session/ClientSessionImpl.java | 57 +++++++++------------- 1 file changed, 24 insertions(+), 33 deletions(-) diff --git a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java index 9bc68a9..a7c07c9 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java @@ -19,18 +19,17 @@ package org.apache.sshd.client.session; import java.io.IOException; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import org.apache.sshd.client.ClientFactoryManager; import org.apache.sshd.client.future.AuthFuture; @@ -60,11 +59,8 @@ public class ClientSessionImpl extends AbstractClientSession { */ private volatile AuthFuture authFuture; - /** Records exceptions before there is an authFuture. */ - private List<Throwable> earlyErrors = new ArrayList<>(); - - /** Guards setting an earlyError and the authFuture together. */ - private final Object errorLock = new Object(); + /** Also guards setting an earlyError and the authFuture together. */ + private final AtomicReference<Throwable> authErrorHolder = new AtomicReference<>(); /** * For clients to store their own metadata @@ -134,33 +130,27 @@ public class ClientSessionImpl extends AbstractClientSession { ClientUserAuthService authService = getUserAuthService(); String serviceName = nextServiceName(); - List<Throwable> errors; + Throwable earlyError; AuthFuture future; // Guard both getting early errors and setting authFuture - synchronized (errorLock) { + synchronized (authErrorHolder) { future = ValidateUtils.checkNotNull( authService.auth(serviceName), "No auth future generated by service=%s", serviceName); - errors = earlyErrors; - earlyErrors = null; + earlyError = authErrorHolder.getAndSet(null); authFuture = future; } - if (errors != null && !errors.isEmpty()) { - Iterator<Throwable> iter = errors.iterator(); - Throwable first = iter.next(); - iter.forEachRemaining(t -> { - if (t != first && t != null) { - first.addSuppressed(t); - } - }); - future.setException(first); + + if (earlyError != null) { + future.setException(earlyError); if (log.isDebugEnabled()) { - log.debug("auth({}) early exception type={}: {}", //$NON-NLS-1$ - this, first.getClass().getSimpleName(), - first.getMessage()); + log.debug("auth({}) early exception type={}: {}", + this, earlyError.getClass().getSimpleName(), + earlyError.getMessage()); } - // Or throw directly: - // throw new IOException(first.getMessage(), first); + // TODO consider throw directly: + // throw new IOException(earlyError.getMessage(), earlyError); } + return future; } @@ -184,22 +174,23 @@ public class ClientSessionImpl extends AbstractClientSession { protected void signalAuthFailure(Throwable t) { AuthFuture future = authFuture; + boolean firstError = false; if (future == null) { - synchronized (errorLock) { - if (earlyErrors != null) { - earlyErrors.add(t); - } + synchronized (authErrorHolder) { + // save only the 1st newly signaled exception + firstError = authErrorHolder.compareAndSet(null, t); future = authFuture; } } + if (future != null) { future.setException(t); } + if (log.isDebugEnabled()) { - boolean signalled = future != null && t == future.getException(); - log.debug("signalAuthFailure({}) type={}, signalled={}: {}", this, - t.getClass().getSimpleName(), Boolean.valueOf(signalled), - t.getMessage()); + boolean signalled = (future != null) && (t == future.getException()); + log.debug("signalAuthFailure({}) type={}, signalled={}, first={}: {}", + this, t.getClass().getSimpleName(), signalled, firstError, t.getMessage()); } }
