Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #157 from oleg-nenashev/bug/findbugs_ExportTable
[JENKINS-37566] - FindBugs - Cleanup issues in ExportTable implementation
  • Loading branch information
oleg-nenashev committed Mar 9, 2017
2 parents 865a3f7 + 25aea63 commit 02958b3
Showing 1 changed file with 28 additions and 4 deletions.
32 changes: 28 additions & 4 deletions src/main/java/hudson/remoting/ExportTable.java
Expand Up @@ -23,6 +23,7 @@
*/ */
package hudson.remoting; package hudson.remoting;


import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import javax.annotation.Nonnull; import javax.annotation.Nonnull;
import java.io.IOException; import java.io.IOException;
import java.io.PrintWriter; import java.io.PrintWriter;
Expand All @@ -39,6 +40,10 @@


import static java.util.logging.Level.*; import static java.util.logging.Level.*;
import javax.annotation.CheckForNull; 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. * Manages unique ID for exported objects, and allows look-up from IDs.
Expand Down Expand Up @@ -225,11 +230,26 @@ static class Source extends Exception {
* Optional location that indicates where the actual call site was that triggered the activity, * 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. * in case it was requested from the other side of the channel.
*/ */
@SuppressWarnings("ResultOfMethodCallIgnored")
Source(@CheckForNull Throwable callSite) { Source(@CheckForNull Throwable callSite) {
super(callSite); super(callSite);
// force the computation of the stack trace in a Java friendly data structure, updateOurStackTraceCache();
// so that the call stack can be seen from the heap dump after the fact. }
getStackTrace();
// 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();
} }
} }


Expand Down Expand Up @@ -257,10 +277,14 @@ public String toString() {


/** /**
* Captures the list of export, so that they can be unexported later. * Captures the list of export, so that they can be unexported later.
*
* This is tied to a particular thread, so it only records operations * This is tied to a particular thread, so it only records operations
* on the current thread. * 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<Entry> { public final class ExportList extends ArrayList<Entry> {
private final ExportList old; private final ExportList old;
private ExportList() { private ExportList() {
Expand Down

0 comments on commit 02958b3

Please sign in to comment.