From 07247205cd800b300323f3e6a136c3baf76a6575 Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Mon, 6 Mar 2017 11:52:51 +0100 Subject: [PATCH] Cleanup hudson.remoting.Channel.Ref - NPE et al. This is an aggregated PR, cleans up the class implementation - [x] Fix the NPE risk introduced by @kohsuke in https://github.com/jenkinsci/remoting/commit/0d8a2af7cffa6aa5a6f2675e810b286a984af04e - [x] Add name to toString() in order to display names for terminated channels - [x] Update Javadoc and annotations https://github.com/jenkinsci/remoting/commit/0d8a2af7cffa6aa5a6f2675e810b286a984af04e also introduces the incompatible change of Ref#clear(), but it's fine this the class is package-internal --- src/main/java/hudson/remoting/Channel.java | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/main/java/hudson/remoting/Channel.java b/src/main/java/hudson/remoting/Channel.java index 05124e766..97f618ef9 100644 --- a/src/main/java/hudson/remoting/Channel.java +++ b/src/main/java/hudson/remoting/Channel.java @@ -876,7 +876,7 @@ protected V adapt(UserResponse r) throws ExecutionException { */ @java.lang.SuppressWarnings("ToArrayCallWithZeroLengthArrayArgument") @SuppressWarnings("ITA_INEFFICIENT_TO_ARRAY") // intentionally; race condition on listeners otherwise - public void terminate(IOException e) { + public void terminate(@Nonnull IOException e) { try { synchronized (this) { if (e == null) throw new IllegalArgumentException(); @@ -1707,12 +1707,14 @@ protected int[] initialValue() { /** * A reference for the {@link Channel} that can be cleared out on {@link #close()}/{@link #terminate(IOException)}. * Could probably be replaced with {@link AtomicReference} but then we would not retain the only change being - * from valid channel to {@code null} channel symmantics of this class. - * @since FIXME after merge + * from valid channel to {@code null} channel semantics of this class. + * @since 2.52 * @see #reference */ /*package*/ static final class Ref { + /** + * Cached name of the channel. * @see {@link Channel#getName()} */ @Nonnull @@ -1734,7 +1736,7 @@ protected int[] initialValue() { * @param channel the {@link Channel}. */ private Ref(@CheckForNull Channel channel) { - this.name = channel.getName(); + this.name = channel != null ? channel.getName() : "unknown (null reference)"; this.channel = channel; } @@ -1749,13 +1751,19 @@ public Channel channel() { /** * If the channel is null, return the cause of the channel termination. + * @return Cause or {@code null} if it is not available + * @since 3.1 */ + @CheckForNull public Exception cause() { return cause; } /** + * Returns the cached name of the channel. + * @return Channel name or {@code "unknown (null reference)"} if the reference has been initialized by {@code null} {@link Channel} instance. * @see Channel#getName() + * @since 3.1 */ @Nonnull public String name() { @@ -1765,8 +1773,9 @@ public String name() { /** * Clears the {@link #channel} to signify that the {@link Channel} has been closed and break any complex * object cycles that might prevent the full garbage collection of the channel's associated object tree. + * @param cause Channel termination cause */ - public void clear(Exception cause) { + public void clear(@Nonnull Exception cause) { this.channel = null; this.cause = cause; } @@ -1795,6 +1804,7 @@ public int hashCode() { public String toString() { final StringBuilder sb = new StringBuilder("Channel.Ref{"); sb.append("channel=").append(channel); + sb.append(",name=").append(name); sb.append('}'); return sb.toString(); }