From 0cc7e939c3632fb3de968536878ee15b6ed70d51 Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Tue, 7 Mar 2017 16:50:38 +0100 Subject: [PATCH 1/2] Findbugs - Fix RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT for ExportTable.Source --- .../java/hudson/remoting/ExportTable.java | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/main/java/hudson/remoting/ExportTable.java b/src/main/java/hudson/remoting/ExportTable.java index 9f3fc82e9..79ab27dc9 100644 --- a/src/main/java/hudson/remoting/ExportTable.java +++ b/src/main/java/hudson/remoting/ExportTable.java @@ -39,6 +39,8 @@ import static java.util.logging.Level.*; import javax.annotation.CheckForNull; +import javax.annotation.CheckReturnValue; +import javax.annotation.meta.When; /** * Manages unique ID for exported objects, and allows look-up from IDs. @@ -225,11 +227,26 @@ static class Source extends Exception { * Optional location that indicates where the actual call site was that triggered the activity, * in case it was requested from the other side of the channel. */ + @SuppressWarnings("ResultOfMethodCallIgnored") Source(@CheckForNull Throwable callSite) { super(callSite); - // force the computation of the stack trace in a Java friendly data structure, - // so that the call stack can be seen from the heap dump after the fact. - getStackTrace(); + updateOurStackTraceCache(); + } + + // TODO: We export the objects frequently, The current approach ALWAYS leads + // to creation of two Stacktrace arrays in the memory: the original and the cloned one + // Throwable API. Throwable API allows to workaround it only by using a heavy printStackTrace() method. + // Approach #1: Maybe a manual implementation of getOurStackTrace() and local storage is preferable. + // Approach #2: Consider disabling this logic by default + /** + * Update the internal stacktrace cache. + * Forces the computation of the stack trace in a Java friendly data structure, + * so that the call stack can be seen from the heap dump after the fact. + * @return Cloned version of the inner cache. + */ + @CheckReturnValue(when = When.NEVER) + protected final StackTraceElement[] updateOurStackTraceCache() { + return getStackTrace(); } } From 25aea63a112b7fb9bba3ebf80b29e1e097aceb62 Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Tue, 7 Mar 2017 17:08:21 +0100 Subject: [PATCH 2/2] FindBugs - Suppress ExportList serialization warning since it is not a part of public API --- src/main/java/hudson/remoting/ExportTable.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/java/hudson/remoting/ExportTable.java b/src/main/java/hudson/remoting/ExportTable.java index 79ab27dc9..45747895e 100644 --- a/src/main/java/hudson/remoting/ExportTable.java +++ b/src/main/java/hudson/remoting/ExportTable.java @@ -23,6 +23,7 @@ */ package hudson.remoting; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import javax.annotation.Nonnull; import java.io.IOException; import java.io.PrintWriter; @@ -41,6 +42,8 @@ import javax.annotation.CheckForNull; import javax.annotation.CheckReturnValue; import javax.annotation.meta.When; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; /** * Manages unique ID for exported objects, and allows look-up from IDs. @@ -274,10 +277,14 @@ public String toString() { /** * Captures the list of export, so that they can be unexported later. - * * This is tied to a particular thread, so it only records operations * on the current thread. + * The class is not serializable. */ + @Restricted(NoExternalUse.class) + @SuppressFBWarnings(value = "SE_BAD_FIELD_INNER_CLASS", + justification = "ExportList is supposed to be serializable as ArrayList, but it is not. " + + "The issue is ignored since the class does not belong to the public API") public final class ExportList extends ArrayList { private final ExportList old; private ExportList() {