Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup hudson.remoting.Channel.Ref - NPE et al. #153

Merged
merged 1 commit into from
Mar 7, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions src/main/java/hudson/remoting/Channel.java
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,7 @@ protected V adapt(UserResponse<V,T> 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();
Expand Down Expand Up @@ -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
Expand All @@ -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)";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where was FindBugs before?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FindBugs prints warnings, but it does not fail the build now. I am handling it as https://issues.jenkins-ci.org/browse/JENKINS-37566 . One year ago there were more than 100 failures, once I merge the pending PRs there should be only 10 left

this.channel = channel;
}

Expand All @@ -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() {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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();
}
Expand Down