Polish gh-4857 (#5368)

Author: izeye

concurrency-tests/build.gradle

@@ -11,7 +11,7 @@ dependencies {
 }
 
 jcstress {
-    jcstressDependency 'org.openjdk.jcstress:jcstress-core:0.16'
+    libs.jcstressCore
 
     verbose = true
 }

micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java

@@ -103,8 +103,8 @@ public abstract class MeterRegistry {
 
     /**
      * write/remove guarded by meterMapLock, read in
-     * {@link this#getOrCreateMeter(DistributionStatisticConfig, BiFunction, Id,
-     * Function)} is unguarded
+     * {@link #getOrCreateMeter(DistributionStatisticConfig, BiFunction, Id, Function)} is
+     * unguarded
      */
     private final Map<Id, Meter> preFilterIdToMeterMap = new HashMap<>();
 
@@ -755,15 +755,16 @@ public Meter remove(Meter.Id mappedId) {
         if (meterMap.containsKey(mappedId)) {
             synchronized (meterMapLock) {
                 final Meter removedMeter = meterMap.remove(mappedId);
-                Iterator<Map.Entry<Id, Meter>> iterator = preFilterIdToMeterMap.entrySet().iterator();
-                while (iterator.hasNext()) {
-                    Map.Entry<Id, Meter> nextEntry = iterator.next();
-                    if (nextEntry.getValue().equals(removedMeter)) {
-                        stalePreFilterIds.remove(nextEntry.getKey());
-                        iterator.remove();
-                    }
-                }
                 if (removedMeter != null) {
+                    Iterator<Map.Entry<Id, Meter>> iterator = preFilterIdToMeterMap.entrySet().iterator();
+                    while (iterator.hasNext()) {
+                        Map.Entry<Id, Meter> nextEntry = iterator.next();
+                        if (nextEntry.getValue().equals(removedMeter)) {
+                            stalePreFilterIds.remove(nextEntry.getKey());
+                            iterator.remove();
+                        }
+                    }
+
                     Set<Id> synthetics = syntheticAssociations.remove(mappedId);
                     if (synthetics != null) {
                         for (Id synthetic : synthetics) {

micrometer-core/src/test/java/io/micrometer/core/instrument/MeterRegistryTest.java

@@ -272,7 +272,6 @@ void differentPreFilterIdsMapToSameIdWithStaleId() {
 
         assertThat(c1).isSameAs(c2);
         assertThat(registry.remove(c1)).isSameAs(c2);
-        assertThat(registry.remove(c2)).isNull();
         assertThat(registry.getMeters()).isEmpty();
     }
 
@@ -314,6 +313,7 @@ void removingStaleMeterRemovesItFromAllInternalState() {
         Counter c1 = registry.counter("counter");
         // make c1 marked as stale
         registry.config().commonTags("common", "tag");
+        assertThat(registry._getStalePreFilterIds()).hasSize(1);
 
         registry.remove(c1.getId());
         assertThat(registry.getMeters()).isEmpty();
@@ -338,6 +338,7 @@ void unchangedStaleMeterShouldBeUnmarked() {
         Counter c1 = registry.counter("counter");
         // make c1 stale
         registry.config().meterFilter(MeterFilter.ignoreTags("abc"));
+        assertThat(registry._getStalePreFilterIds()).hasSize(1);
         // this should cause c1 (== c2) to be unmarked as stale
         Counter c2 = registry.counter("counter");
 
@@ -347,7 +348,7 @@ void unchangedStaleMeterShouldBeUnmarked() {
         assertThat(registry._getPreFilterIdToMeterMap()).hasSize(1);
         assertThat(registry._getStalePreFilterIds())
             .describedAs("If the meter-filter doesn't alter the meter creation, meters are never unmarked "
-                    + "from staleness and we end-up paying the additional cost everytime")
+                    + "from staleness and we end up paying the additional cost every time")
             .isEmpty();
     }