Fix thread-safety issue in double-checked singleton instance creation.

Author: phipag

powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/internal/LambdaLoggingAspect.java

@@ -312,8 +312,8 @@ private void setCorrelationIdFromNode(String correlationIdPath, JsonNode jsonNod
 
     private byte[] bytesFromInputStreamSafely(final InputStream inputStream) throws IOException {
         try (ByteArrayOutputStream out = new ByteArrayOutputStream();
-                InputStreamReader reader = new InputStreamReader(inputStream, UTF_8)) {
-            OutputStreamWriter writer = new OutputStreamWriter(out, UTF_8);
+                InputStreamReader reader = new InputStreamReader(inputStream, UTF_8);
+                OutputStreamWriter writer = new OutputStreamWriter(out, UTF_8)) {
             int n;
             char[] buffer = new char[4096];
             while (-1 != (n = reader.read(buffer))) {
@@ -326,7 +326,7 @@ private byte[] bytesFromInputStreamSafely(final InputStream inputStream) throws
 
     private OutputStream prepareOutputStreamForLogging(Logging logging,
             Object[] proceedArgs) {
-        if ((logging.logResponse() || POWERTOOLS_LOG_RESPONSE)) {
+        if (logging.logResponse() || POWERTOOLS_LOG_RESPONSE) {
             OutputStream backupOutputStream = (OutputStream) proceedArgs[1];
             proceedArgs[1] = new ByteArrayOutputStream();
             return backupOutputStream;

powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/internal/LoggingManagerRegistry.java

@@ -20,15 +20,16 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.ServiceLoader;
-import java.util.concurrent.atomic.AtomicReference;
 
 /**
  * Thread-safe singleton registry for LoggingManager instances.
  * Handles lazy loading and caching of the LoggingManager implementation.
  */
 public final class LoggingManagerRegistry {
 
-    private static final AtomicReference<LoggingManager> instance = new AtomicReference<>();
+    // Used with double-checked locking within getLoggingManger()
+    @SuppressWarnings("java:S3077")
+    private static volatile LoggingManager instance;
 
     private LoggingManagerRegistry() {
         // Utility class
@@ -40,13 +41,13 @@ private LoggingManagerRegistry() {
      * @return the LoggingManager instance
      */
     public static LoggingManager getLoggingManager() {
-        LoggingManager manager = instance.get();
+        LoggingManager manager = instance;
         if (manager == null) {
             synchronized (LoggingManagerRegistry.class) {
-                manager = instance.get();
+                manager = instance;
                 if (manager == null) {
                     manager = loadLoggingManager();
-                    instance.set(manager);
+                    instance = manager;
                 }
             }
         }

powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/internal/KeyBufferTest.java

@@ -248,16 +248,18 @@ void shouldBeThreadSafeForDifferentKeys() throws InterruptedException {
             });
         }
 
-        assertThat(latch.await(5, TimeUnit.SECONDS)).isTrue();
-
-        // Verify each key has its events
-        for (int i = 0; i < threadCount; i++) {
-            String key = "key" + i;
-            Deque<String> events = buffer.removeAll(key);
-            assertThat(events).isNotNull().isNotEmpty();
+        try {
+            assertThat(latch.await(5, TimeUnit.SECONDS)).isTrue();
+
+            // Verify each key has its events
+            for (int i = 0; i < threadCount; i++) {
+                String key = "key" + i;
+                Deque<String> events = buffer.removeAll(key);
+                assertThat(events).isNotNull().isNotEmpty();
+            }
+        } finally {
+            executor.shutdown();
         }
-
-        executor.shutdown();
     }
 
     @Test
@@ -281,12 +283,14 @@ void shouldBeThreadSafeForSameKey() throws InterruptedException {
             });
         }
 
-        assertThat(latch.await(5, TimeUnit.SECONDS)).isTrue();
-
-        Deque<String> events = buffer.removeAll("sharedKey");
-        assertThat(events).isNotNull().isNotEmpty();
+        try {
+            assertThat(latch.await(5, TimeUnit.SECONDS)).isTrue();
 
-        executor.shutdown();
+            Deque<String> events = buffer.removeAll("sharedKey");
+            assertThat(events).isNotNull().isNotEmpty();
+        } finally {
+            executor.shutdown();
+        }
     }
 
     @Test

powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/internal/LoggingManagerRegistryTest.java

@@ -131,8 +131,11 @@ void testGetLoggingManager_shouldBeThreadSafe() throws InterruptedException {
         }
 
         // THEN
-        latch.await(5, TimeUnit.SECONDS);
-        executor.shutdown();
-        assertThat(sharedInstance.get()).isNotNull();
+        try {
+            latch.await(5, TimeUnit.SECONDS);
+            assertThat(sharedInstance.get()).isNotNull();
+        } finally {
+            executor.shutdown();
+        }
     }
 }