fix: Try fixing spotted bugs

Author: bric3

communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java

@@ -11,6 +11,7 @@
 import datadog.remoteconfig.DefaultConfigurationPoller;
 import datadog.trace.api.Config;
 import datadog.trace.util.AgentTaskScheduler;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.security.Security;
 import java.util.ArrayList;
 import java.util.List;
@@ -27,9 +28,15 @@ public class SharedCommunicationObjects {
   private final List<Runnable> pausedComponents = new ArrayList<>();
   private volatile boolean paused;
 
+  @SuppressFBWarnings("PA_PUBLIC_PRIMITIVE_ATTRIBUTE")
   public OkHttpClient okHttpClient;
+
+  @SuppressFBWarnings("PA_PUBLIC_PRIMITIVE_ATTRIBUTE")
   public HttpUrl agentUrl;
+
+  @SuppressFBWarnings("PA_PUBLIC_PRIMITIVE_ATTRIBUTE")
   public Monitoring monitoring;
+
   private volatile DDAgentFeaturesDiscovery featuresDiscovery;
   private ConfigurationPoller configurationPoller;
 

communication/src/main/java/datadog/communication/monitor/DDAgentStatsDClientManager.java

@@ -14,6 +14,8 @@
 public final class DDAgentStatsDClientManager implements StatsDClientManager {
   private static final DDAgentStatsDClientManager INSTANCE = new DDAgentStatsDClientManager();
 
+  private DDAgentStatsDClientManager() {}
+
   private static final boolean USE_LOGGING_CLIENT =
       LOGGING_WRITER_TYPE.equals(Config.get().getWriterType());
 

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java

@@ -56,6 +56,7 @@
 import datadog.trace.util.AgentTaskScheduler;
 import datadog.trace.util.AgentThreadFactory.AgentThread;
 import datadog.trace.util.throwable.FatalAgentMisconfigurationError;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.lang.instrument.Instrumentation;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
@@ -189,6 +190,7 @@ public boolean isEnabledByDefault() {
    * <p>The Agent is considered to start successfully if Instrumentation can be activated. All other
    * pieces are considered optional.
    */
+  @SuppressFBWarnings("AT_STALE_THREAD_WRITE_OF_PRIMITIVE")
   public static void start(
       final Object bootstrapInitTelemetry,
       final Instrumentation inst,
@@ -436,6 +438,7 @@ private static void injectAgentArgsConfig(String agentArgs) {
     }
   }
 
+  @SuppressFBWarnings("AT_STALE_THREAD_WRITE_OF_PRIMITIVE")
   private static void configureCiVisibility(URL agentJarURL) {
     // Retro-compatibility for the old way to configure CI Visibility
     if ("true".equals(ddGetProperty("dd.integration.junit.enabled"))

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/PatchLogger.java

@@ -29,7 +29,7 @@ public static PatchLogger getAnonymousLogger(final String resourceBundleName) {
     return SAFE_LOGGER;
   }
 
-  protected PatchLogger(final String name, final String resourceBundleName) {
+  private PatchLogger(final String name, final String resourceBundleName) {
     // super(name, resourceBundleName);
   }
 

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jms/SessionState.java

@@ -2,7 +2,6 @@
 
 import datadog.trace.api.Config;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.ArrayDeque;
 import java.util.Collections;
 import java.util.Comparator;
@@ -65,7 +64,6 @@ public int compare(Map.Entry<Thread, TimeInQueue> o1, Map.Entry<Thread, TimeInQu
   private volatile int timeInQueueSpanCount = 0;
 
   // this field is protected by synchronization of capturedSpans, but SpotBugs miss that
-  @SuppressFBWarnings("IS2_INCONSISTENT_SYNC")
   private boolean capturingFlipped = false;
 
   public SessionState(int ackMode, boolean timeInQueueEnabled) {

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java

@@ -3,15 +3,16 @@
 import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
 import datadog.trace.api.civisibility.execution.TestStatus;
 import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 
 /** Retries a test case if it failed, up to a maximum number of times. */
 public class RetryUntilSuccessful implements TestExecutionPolicy {
 
   private final int maxExecutions;
   private final boolean suppressFailures;
-  private int executions;
-  private boolean successfulExecutionSeen;
+  private final AtomicInteger executions = new AtomicInteger(0);
+  private final AtomicBoolean successfulExecutionSeen = new AtomicBoolean(false);
 
   /** Total retry counter that is shared by all retry until successful policies (currently ATR) */
   private final AtomicInteger totalRetryCount;
@@ -21,29 +22,29 @@ public RetryUntilSuccessful(
     this.maxExecutions = maxExecutions;
     this.suppressFailures = suppressFailures;
     this.totalRetryCount = totalRetryCount;
-    this.executions = 0;
   }
 
   @Override
   public ExecutionOutcome registerExecution(TestStatus status, long durationMillis) {
-    ++executions;
-    successfulExecutionSeen |= (status != TestStatus.fail);
-    if (executions > 1) {
+    int execs = executions.incrementAndGet();
+    boolean success = successfulExecutionSeen.get() | (status != TestStatus.fail);
+    successfulExecutionSeen.set(success);
+    if (execs > 1) {
       totalRetryCount.incrementAndGet();
     }
 
     boolean lastExecution = !retriesLeft();
-    boolean retry = executions > 1; // first execution is not a retry
+    boolean retry = execs > 1; // first execution is not a retry
     return new ExecutionOutcomeImpl(
         status == TestStatus.fail && (!lastExecution || suppressFailures),
         lastExecution,
-        lastExecution && !successfulExecutionSeen,
+        lastExecution && !success,
         false,
         retry ? RetryReason.atr : null);
   }
 
   private boolean retriesLeft() {
-    return !successfulExecutionSeen && executions < maxExecutions;
+    return !successfulExecutionSeen.get() && executions.get() < maxExecutions;
   }
 
   @Override
@@ -57,7 +58,7 @@ public boolean applicable() {
   public boolean suppressFailures() {
     // do not suppress failures for last execution (unless flag to suppress all failures is set);
     // the +1 is because this method is called _before_ subsequent execution is registered
-    return executions + 1 < maxExecutions || suppressFailures;
+    return executions.get() + 1 < maxExecutions || suppressFailures;
   }
 
   @Override

dd-java-agent/agent-iast/iast-test-fixtures/src/main/groovy/com/datadog/iast/test/IastRequestTestRunner.groovy

@@ -6,11 +6,13 @@ import datadog.trace.agent.test.utils.OkHttpUtils
 import datadog.trace.api.gateway.IGSpanInfo
 import datadog.trace.api.gateway.RequestContext
 import datadog.trace.api.gateway.RequestContextSlot
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings
 import okhttp3.OkHttpClient
 
 import java.util.concurrent.LinkedBlockingQueue
 import java.util.concurrent.TimeUnit
 
+@SuppressFBWarnings("HSM_HIDING_METHOD")
 class IastRequestTestRunner extends IastAgentTestRunner implements IastRequestContextPreparationTrait {
 
   private static final LinkedBlockingQueue<TaintedObjectCollection> TAINTED_OBJECTS = new LinkedBlockingQueue<>()

dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastSystem.java

@@ -55,6 +55,7 @@
 import datadog.trace.api.iast.telemetry.Verbosity;
 import datadog.trace.util.AgentTaskScheduler;
 import datadog.trace.util.stacktrace.StackWalkerFactory;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.lang.instrument.Instrumentation;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.UndeclaredThrowableException;
@@ -70,7 +71,11 @@
 public class IastSystem {
 
   private static final Logger LOGGER = LoggerFactory.getLogger(IastSystem.class);
+
+  @SuppressFBWarnings("PA_PUBLIC_PRIMITIVE_ATTRIBUTE")
   public static boolean DEBUG = false;
+
+  @SuppressFBWarnings("PA_PUBLIC_PRIMITIVE_ATTRIBUTE")
   public static Verbosity VERBOSITY = Verbosity.OFF;
 
   public static void start(final SubscriptionService ss) {

dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/StringModuleImpl.java

@@ -224,7 +224,6 @@ public void onStringConcatFactory(
   }
 
   @Override
-  @SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ")
   public void onStringSubSequence(
       @Nonnull CharSequence self, int beginIndex, int endIndex, @Nullable CharSequence result) {
     if (self == result || !canBeTainted(result)) {
@@ -722,7 +721,6 @@ public void onStringReplace(
 
   /** This method is used to make an {@code CallSite.Around} of the {@code String.replace} method */
   @Override
-  @SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ")
   public String onStringReplace(
       @Nonnull String self, CharSequence oldCharSeq, CharSequence newCharSeq) {
     final IastContext ctx = IastContext.Provider.get();
@@ -805,7 +803,6 @@ public String onStringReplace(
   }
 
   @Override
-  @SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ")
   public void onStringValueOf(Object param, @Nonnull String result) {
     if (param == null || !canBeTainted(result)) {
       return;

dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedMap.java

@@ -102,7 +102,7 @@ class TaintedMapImpl implements TaintedMap, Runnable {
     /**
      * Flag for the current alive tainted objects (red/black style marking for max age calculation).
      */
-    protected boolean generation;
+    protected volatile boolean generation;
 
     /** Whether to collect the {@link IastMetric#TAINTED_FLAT_MODE} metric or not */
     protected boolean collectFlatBucketMetric;