From f2da4d29beba175bf92547689d680d3e996bba07 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Wed, 1 Jul 2026 22:09:45 +0100 Subject: [PATCH] Update build plugins and address new spotbugs findings --- .../instrument/classmatch/ClassFile.java | 49 ++++++------ .../fieldinject/GlobalObjectStore.java | 75 ++++++++++--------- gradle/libs.versions.toml | 8 +- gradle/wrapper/gradle-wrapper.properties | 4 +- .../instrument/utils/ClassLoaderValue.java | 2 + 5 files changed, 74 insertions(+), 64 deletions(-) diff --git a/class-match/src/main/java/datadog/instrument/classmatch/ClassFile.java b/class-match/src/main/java/datadog/instrument/classmatch/ClassFile.java index 1a9904a..f621f20 100644 --- a/class-match/src/main/java/datadog/instrument/classmatch/ClassFile.java +++ b/class-match/src/main/java/datadog/instrument/classmatch/ClassFile.java @@ -41,6 +41,7 @@ public final class ClassFile { private static final byte[] RUNTIME_ANNOTATIONS = "RuntimeVisibleAnnotations".getBytes(US_ASCII); // reduce size of outlines by only extracting interesting annotations + private static final Object annotationsLock = new Object(); private static final Map annotationKeys = new HashMap<>(); private static volatile Map annotationsOfInterest; @@ -95,17 +96,19 @@ public static ClassOutline outline(byte[] bytecode, int offset) { * * @param internalName the annotation type in internal form */ - public static synchronized void annotationOfInterest(String internalName) { - if (annotationKeys.containsKey(internalName)) { - return; // already flagged as interesting - } - Map ofInterest = new HashMap<>(); - if (annotationsOfInterest != null) { - ofInterest.putAll(annotationsOfInterest); // copy on write + public static void annotationOfInterest(String internalName) { + synchronized (annotationsLock) { + if (annotationKeys.containsKey(internalName)) { + return; // already flagged as interesting + } + Map ofInterest = new HashMap<>(); + if (annotationsOfInterest != null) { + ofInterest.putAll(annotationsOfInterest); // copy on write + } + ofInterest.put( + annotationKeys.computeIfAbsent(internalName, ClassFile::annotationKey), internalName); + annotationsOfInterest = ofInterest; } - ofInterest.put( - annotationKeys.computeIfAbsent(internalName, ClassFile::annotationKey), internalName); - annotationsOfInterest = ofInterest; } /** @@ -115,19 +118,21 @@ public static synchronized void annotationOfInterest(String internalName) { * * @param internalNames the annotation types in internal form */ - public static synchronized void annotationsOfInterest(Collection internalNames) { - if (annotationKeys.keySet().containsAll(internalNames)) { - return; // already flagged as interesting - } - Map ofInterest = new HashMap<>(); - if (annotationsOfInterest != null) { - ofInterest.putAll(annotationsOfInterest); // copy on write - } - for (String internalName : internalNames) { - ofInterest.put( - annotationKeys.computeIfAbsent(internalName, ClassFile::annotationKey), internalName); + public static void annotationsOfInterest(Collection internalNames) { + synchronized (annotationsLock) { + if (annotationKeys.keySet().containsAll(internalNames)) { + return; // already flagged as interesting + } + Map ofInterest = new HashMap<>(); + if (annotationsOfInterest != null) { + ofInterest.putAll(annotationsOfInterest); // copy on write + } + for (String internalName : internalNames) { + ofInterest.put( + annotationKeys.computeIfAbsent(internalName, ClassFile::annotationKey), internalName); + } + annotationsOfInterest = ofInterest; } - annotationsOfInterest = ofInterest; } /** Create "modified-UTF8" key to make it easier to match annotations. */ diff --git a/field-inject/src/main/java/datadog/instrument/fieldinject/GlobalObjectStore.java b/field-inject/src/main/java/datadog/instrument/fieldinject/GlobalObjectStore.java index 833384c..8dc7c8e 100644 --- a/field-inject/src/main/java/datadog/instrument/fieldinject/GlobalObjectStore.java +++ b/field-inject/src/main/java/datadog/instrument/fieldinject/GlobalObjectStore.java @@ -35,6 +35,8 @@ public final class GlobalObjectStore { /** Threshold at which we start sampling keys to track old content. */ private static final int OLD_KEYS_THRESHOLD = 512; + private static final Object staleEntriesLock = new Object(); + private static final Map weakMap = new ConcurrentHashMap<>(); private static final Set oldKeys = new HashSet<>(); @@ -51,55 +53,56 @@ private GlobalObjectStore() {} * * @return the estimated remaining size of the global object-store */ - public static synchronized int removeStaleEntries() { - - Map weakMap = GlobalObjectStore.weakMap; - int estimatedSize = weakMap.size(); // capture size before any cleanup - StoreKey key; - while ((key = StoreKey.pollStaleKeys()) != null) { - if (weakMap.remove(key) != null) { - estimatedSize--; + public static int removeStaleEntries() { + synchronized (staleEntriesLock) { + Map weakMap = GlobalObjectStore.weakMap; + int estimatedSize = weakMap.size(); // capture size before any cleanup + StoreKey key; + while ((key = StoreKey.pollStaleKeys()) != null) { + if (weakMap.remove(key) != null) { + estimatedSize--; + } } - } - // The following code handles proactively removing old content in an attempt to guide the - // store below its soft limit. We remove older objects before recent additions, assuming - // that older objects are less likely to be used. For performance reasons this only runs - // after observed periods of growth or reduction, or if the store is near its hard limit. + // The following code handles proactively removing old content in an attempt to guide the + // store below its soft limit. We remove older objects before recent additions, assuming + // that older objects are less likely to be used. For performance reasons this only runs + // after observed periods of growth or reduction, or if the store is near its hard limit. - // We deliberately avoid tracking exact age, and instead regularly sample keys to maintain - // a small set that we know are still alive after a couple of calls to removeStaleEntries. + // We deliberately avoid tracking exact age, and instead regularly sample keys to maintain + // a small set that we know are still alive after a couple of calls to removeStaleEntries. - if (Math.abs(estimatedSize - previousEstimate) > OLD_KEYS_THRESHOLD - || estimatedSize >= (GLOBAL_HARD_LIMIT + GLOBAL_SOFT_LIMIT) / 2) { + if (Math.abs(estimatedSize - previousEstimate) > OLD_KEYS_THRESHOLD + || estimatedSize >= (GLOBAL_HARD_LIMIT + GLOBAL_SOFT_LIMIT) / 2) { - if (estimatedSize >= GLOBAL_SOFT_LIMIT) { - // start proactively removing old content to keep growth in check - for (StoreKey oldKey : oldKeys) { - if (weakMap.remove(oldKey) != null) { - estimatedSize--; + if (estimatedSize >= GLOBAL_SOFT_LIMIT) { + // start proactively removing old content to keep growth in check + for (StoreKey oldKey : oldKeys) { + if (weakMap.remove(oldKey) != null) { + estimatedSize--; + } } + oldKeys.clear(); + } else { + // have any of the old previously sampled keys been collected? + oldKeys.removeIf(StoreKey::isStale); } - oldKeys.clear(); - } else { - // have any of the old previously sampled keys been collected? - oldKeys.removeIf(StoreKey::isStale); - } - int refill = OLD_KEYS_THRESHOLD - oldKeys.size(); - if (refill > 0) { - // sample of keys at this time, don't need strict age ordering - for (StoreKey sampleKey : weakMap.keySet()) { - if (oldKeys.add(sampleKey) && --refill == 0) { - break; + int refill = OLD_KEYS_THRESHOLD - oldKeys.size(); + if (refill > 0) { + // sample of keys at this time, don't need strict age ordering + for (StoreKey sampleKey : weakMap.keySet()) { + if (oldKeys.add(sampleKey) && --refill == 0) { + break; + } } } + + previousEstimate = estimatedSize; } - previousEstimate = estimatedSize; + return estimatedSize; } - - return estimatedSize; } /** diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 2c89c2b..ad54a82 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -3,10 +3,10 @@ asm = "9.10.1" junit-jupiter = "5.14.4" junit-platform = "1.14.4" assertj = "3.27.7" -spotless = "8.5.1" -spotbugs = "6.5.4" -spotbugs-annotations = "4.9.8" -axion-release = "1.21.1" +spotless = "8.8.0" +spotbugs = "6.5.8" +spotbugs-annotations = "4.10.2" +axion-release = "1.21.2" nexus-publish = "2.0.0" jmh-plugin = "0.7.3" diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index bd82f36..dbe66e1 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,7 +1,7 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionSha256Sum=bafc141b619ad6350fd975fc903156dd5c151998cc8b058e8c1044ab5f7b031f -distributionUrl=https\://services.gradle.org/distributions/gradle-9.5.1-bin.zip +distributionSha256Sum=9c0f7faeeb306cb14e4279a3e084ca6b596894089a0638e68a07c945a32c9e14 +distributionUrl=https\://services.gradle.org/distributions/gradle-9.6.1-bin.zip networkTimeout=10000 retries=0 retryBackOffMs=500 diff --git a/utils/src/main/java/datadog/instrument/utils/ClassLoaderValue.java b/utils/src/main/java/datadog/instrument/utils/ClassLoaderValue.java index 066d22c..387a9bd 100644 --- a/utils/src/main/java/datadog/instrument/utils/ClassLoaderValue.java +++ b/utils/src/main/java/datadog/instrument/utils/ClassLoaderValue.java @@ -11,6 +11,7 @@ import static datadog.instrument.utils.ClassLoaderKey.SYSTEM_CLASS_LOADER; import datadog.instrument.utils.ClassLoaderKey.LookupKey; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Map; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; @@ -45,6 +46,7 @@ public abstract class ClassLoaderValue { private final Map otherValues = new ConcurrentHashMap<>(); /** Register subclass instances for cleaning. */ + @SuppressFBWarnings("CT_CONSTRUCTOR_THROW") // registerCleaner never sees partial instance protected ClassLoaderValue() { ClassLoaderKey.registerCleaner(otherValues::remove); }