From 452d6ca6d653a391565b93317664874bca84d0ac Mon Sep 17 00:00:00 2001 From: Tin Bubanko <52252524+bubanko@users.noreply.github.com> Date: Tue, 27 Oct 2026 16:49:09 +0100 Subject: [PATCH 1/2] fix(batch): use thread-safe collection in MultiThreadMDC Replace `ArrayList` with a thread-safe `Set` implementation to avoid concurrency issues when `MultiThreadMDC` is used in parallel batch processing scenarios. Replace `contains` checks with the corresponding `add` or `remove` calls, since these methods return the information needed and are idempotent. GitHub issue number: #2712 --- .../batch/internal/MultiThreadMDC.java | 14 ++-- .../batch/internal/MultiThreadMDCTest.java | 74 +++++++++++++++++++ 2 files changed, 80 insertions(+), 8 deletions(-) create mode 100644 powertools-batch/src/test/java/software/amazon/lambda/powertools/batch/internal/MultiThreadMDCTest.java diff --git a/powertools-batch/src/main/java/software/amazon/lambda/powertools/batch/internal/MultiThreadMDC.java b/powertools-batch/src/main/java/software/amazon/lambda/powertools/batch/internal/MultiThreadMDC.java index b2b85044b..6934d0ff3 100644 --- a/powertools-batch/src/main/java/software/amazon/lambda/powertools/batch/internal/MultiThreadMDC.java +++ b/powertools-batch/src/main/java/software/amazon/lambda/powertools/batch/internal/MultiThreadMDC.java @@ -14,9 +14,9 @@ package software.amazon.lambda.powertools.batch.internal; -import java.util.ArrayList; -import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.slf4j.MDC; @@ -29,7 +29,7 @@ public class MultiThreadMDC { private static final Logger LOGGER = LoggerFactory.getLogger(MultiThreadMDC.class); - private final List mdcAwareThreads = new ArrayList<>(); + private final Set mdcAwareThreads = ConcurrentHashMap.newKeySet(); private final Map contextMap; public MultiThreadMDC() { @@ -38,17 +38,15 @@ public MultiThreadMDC() { } public void copyMDCToThread(String thread) { - if (!mdcAwareThreads.contains(thread)) { + if (mdcAwareThreads.add(thread)) { LOGGER.debug("Copy MDC to thread {}", thread); MDC.setContextMap(contextMap); - mdcAwareThreads.add(thread); } } public void removeThread(String thread) { - if (mdcAwareThreads.contains(thread)) { - LOGGER.debug("Removing thread {}", thread); - mdcAwareThreads.remove(thread); + if (mdcAwareThreads.remove(thread)) { + LOGGER.debug("Removed thread {}", thread); } } } diff --git a/powertools-batch/src/test/java/software/amazon/lambda/powertools/batch/internal/MultiThreadMDCTest.java b/powertools-batch/src/test/java/software/amazon/lambda/powertools/batch/internal/MultiThreadMDCTest.java new file mode 100644 index 000000000..b0595859f --- /dev/null +++ b/powertools-batch/src/test/java/software/amazon/lambda/powertools/batch/internal/MultiThreadMDCTest.java @@ -0,0 +1,74 @@ +/* + * Copyright 2023 Amazon.com, Inc. or its affiliates. + * Licensed under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ +package software.amazon.lambda.powertools.batch.internal; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThatNoException; + +class MultiThreadMDCTest { + + @Nested + class ConcurrencyTests { + + private static final int REMOVE_THREAD_WORKERS_COUNT = 10; + private static final int COPY_MDC_TO_THREAD_ITERATIONS = 100000; + private static final String THREAD_NAME = "test"; + + @Test + void shouldBeThreadSafe() { + MultiThreadMDC multiThreadMDC = new MultiThreadMDC(); + AtomicBoolean stopSignal = new AtomicBoolean(false); + ExecutorService executor = Executors.newFixedThreadPool(REMOVE_THREAD_WORKERS_COUNT + 1); + try { + List> futures = new ArrayList<>(); + for (int i = 0; i < REMOVE_THREAD_WORKERS_COUNT; i++) { + futures.add(CompletableFuture.runAsync(() -> threadRemover(multiThreadMDC, stopSignal), executor)); + } + futures.add(CompletableFuture.runAsync(() -> mdcCopier(multiThreadMDC, stopSignal), executor)); + assertThatNoException().isThrownBy(() -> futures.forEach(CompletableFuture::join)); + } finally { + executor.shutdown(); + } + } + + private static void mdcCopier(MultiThreadMDC multiThreadMDC, AtomicBoolean stopSignal) { + try { + for (int i = 0; i < COPY_MDC_TO_THREAD_ITERATIONS; i++) { + multiThreadMDC.copyMDCToThread(THREAD_NAME); + } + } finally { + stopSignal.set(true); + } + } + + private static void threadRemover(MultiThreadMDC multiThreadMDC, AtomicBoolean stopSignal) { + try { + while (!stopSignal.get()) { + multiThreadMDC.removeThread(THREAD_NAME); + } + } finally { + stopSignal.set(true); + } + } + } +} From 2a2b74ebafde3abf3cf58b5c29536a1ee8d4c577 Mon Sep 17 00:00:00 2001 From: Tin Bubanko <52252524+bubanko@users.noreply.github.com> Date: Wed, 28 Oct 2026 12:22:04 +0100 Subject: [PATCH 2/2] refactor(batch): make MultiThreadMDC methods more testable Change `copyMDCToThread` and `removeThread` to return boolean values indicating operation success, enabling basic unit testing. Related GitHub issue number: #2712 --- .../batch/internal/MultiThreadMDC.java | 8 ++++-- .../batch/internal/MultiThreadMDCTest.java | 27 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/powertools-batch/src/main/java/software/amazon/lambda/powertools/batch/internal/MultiThreadMDC.java b/powertools-batch/src/main/java/software/amazon/lambda/powertools/batch/internal/MultiThreadMDC.java index 6934d0ff3..e35b6bb3c 100644 --- a/powertools-batch/src/main/java/software/amazon/lambda/powertools/batch/internal/MultiThreadMDC.java +++ b/powertools-batch/src/main/java/software/amazon/lambda/powertools/batch/internal/MultiThreadMDC.java @@ -37,16 +37,20 @@ public MultiThreadMDC() { contextMap = MDC.getCopyOfContextMap(); } - public void copyMDCToThread(String thread) { + public boolean copyMDCToThread(String thread) { if (mdcAwareThreads.add(thread)) { LOGGER.debug("Copy MDC to thread {}", thread); MDC.setContextMap(contextMap); + return true; } + return false; } - public void removeThread(String thread) { + public boolean removeThread(String thread) { if (mdcAwareThreads.remove(thread)) { LOGGER.debug("Removed thread {}", thread); + return true; } + return false; } } diff --git a/powertools-batch/src/test/java/software/amazon/lambda/powertools/batch/internal/MultiThreadMDCTest.java b/powertools-batch/src/test/java/software/amazon/lambda/powertools/batch/internal/MultiThreadMDCTest.java index b0595859f..0035a2e89 100644 --- a/powertools-batch/src/test/java/software/amazon/lambda/powertools/batch/internal/MultiThreadMDCTest.java +++ b/powertools-batch/src/test/java/software/amazon/lambda/powertools/batch/internal/MultiThreadMDCTest.java @@ -23,10 +23,37 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatNoException; class MultiThreadMDCTest { + @Test + void copyMDCToThread_givenMdcNotCopiedToThread_shouldCopyMdcToThread() { + MultiThreadMDC multiThreadMDC = new MultiThreadMDC(); + assertThat(multiThreadMDC.copyMDCToThread("test")).isTrue(); + } + + @Test + void copyMDCToThread_givenMdcAlreadyCopiedToThread_shouldNotCopyMdcToThread() { + MultiThreadMDC multiThreadMDC = new MultiThreadMDC(); + multiThreadMDC.copyMDCToThread("test"); + assertThat(multiThreadMDC.copyMDCToThread("test")).isFalse(); + } + + @Test + void removeThread_givenMdcAlreadyCopiedToThread_shouldRemoveThread() { + MultiThreadMDC multiThreadMDC = new MultiThreadMDC(); + multiThreadMDC.copyMDCToThread("test"); + assertThat(multiThreadMDC.removeThread("test")).isTrue(); + } + + @Test + void removeThread_givenMdcNotCopiedToThread_shouldNotRemoveThread() { + MultiThreadMDC multiThreadMDC = new MultiThreadMDC(); + assertThat(multiThreadMDC.removeThread("test")).isFalse(); + } + @Nested class ConcurrencyTests {