Skip to content

Commit fae4a51

Browse files
committed
Fix Slf4jThreadLocalAccessor with keys for missing values
In case of missing MDC entries, when a scope was closed, the value should be removed. This was not the case and that could lead to leaks of these MDC entries. This change takes the missing previous values into account and clears them from the MDC in case they were set by a snapshot. Fixes #347. Signed-off-by: Dariusz Jędrzejczyk <[email protected]>
1 parent 7f6e811 commit fae4a51

File tree

2 files changed

+68
-2
lines changed

2 files changed

+68
-2
lines changed

context-propagation/src/main/java/io/micrometer/context/integration/Slf4jThreadLocalAccessor.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2024 the original author or authors.
2+
* Copyright 2024-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -127,6 +127,9 @@ public void setValue(Map<String, String> value) {
127127
if (mdcValue != null) {
128128
MDC.put(key, mdcValue);
129129
}
130+
else {
131+
MDC.remove(key);
132+
}
130133
}
131134
}
132135

context-propagation/src/test/java/io/micrometer/context/integration/Slf4jThreadLocalAccessorTests.java

+64-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2024 the original author or authors.
2+
* Copyright 2024-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -106,4 +106,67 @@ void shouldCopySelectedMdcContentsToNewThread() throws InterruptedException {
106106
executorService.shutdown();
107107
}
108108

109+
@Test
110+
void shouldDealWithEmptySelectedValues() throws InterruptedException {
111+
ContextRegistry registry = new ContextRegistry()
112+
.registerThreadLocalAccessor(new Slf4jThreadLocalAccessor("key1", "key2"));
113+
114+
ExecutorService executorService = Executors.newSingleThreadExecutor();
115+
116+
CountDownLatch latch = new CountDownLatch(1);
117+
AtomicReference<String> value1InOtherThread = new AtomicReference<>();
118+
AtomicReference<String> value2InOtherThread = new AtomicReference<>();
119+
AtomicReference<String> value3InOtherThread = new AtomicReference<>();
120+
121+
MDC.put("key1", "value1_1");
122+
MDC.put("key3", "value3_1");
123+
124+
ContextSnapshot snapshot = ContextSnapshotFactory.builder()
125+
.contextRegistry(registry)
126+
.clearMissing(true)
127+
.build()
128+
.captureAll();
129+
130+
executorService.submit(() -> {
131+
try (ContextSnapshot.Scope scope = snapshot.setThreadLocals()) {
132+
value1InOtherThread.set(MDC.get("key1"));
133+
value2InOtherThread.set(MDC.get("key2"));
134+
value3InOtherThread.set(MDC.get("key3"));
135+
}
136+
latch.countDown();
137+
});
138+
139+
latch.await(100, TimeUnit.MILLISECONDS);
140+
141+
assertThat(value1InOtherThread.get()).isEqualTo("value1_1");
142+
assertThat(value2InOtherThread.get()).isNull();
143+
assertThat(value3InOtherThread.get()).isNull();
144+
145+
CountDownLatch latch2 = new CountDownLatch(1);
146+
147+
MDC.remove("key1");
148+
MDC.put("key2", "value2_2");
149+
150+
ContextSnapshot snapshot2 = ContextSnapshotFactory.builder()
151+
.contextRegistry(registry)
152+
.clearMissing(true)
153+
.build()
154+
.captureAll();
155+
executorService.submit(() -> {
156+
try (ContextSnapshot.Scope scope = snapshot2.setThreadLocals()) {
157+
value1InOtherThread.set(MDC.get("key1"));
158+
value2InOtherThread.set(MDC.get("key2"));
159+
value3InOtherThread.set(MDC.get("key3"));
160+
}
161+
latch2.countDown();
162+
});
163+
164+
latch2.await(100, TimeUnit.MILLISECONDS);
165+
166+
assertThat(value1InOtherThread.get()).isNull();
167+
assertThat(value2InOtherThread.get()).isEqualTo("value2_2");
168+
assertThat(value3InOtherThread.get()).isNull();
169+
executorService.shutdown();
170+
}
171+
109172
}

0 commit comments

Comments
 (0)