From 205bf2db165152d0833f3704aca5c0047cf0a5ed Mon Sep 17 00:00:00 2001 From: Tomas Hofman Date: Wed, 27 Sep 2023 12:45:31 +0200 Subject: [PATCH 1/4] UNDERTOW-2309 Prevent memory leak in DefaultByteBufferPool --- .../server/DefaultByteBufferPool.java | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/io/undertow/server/DefaultByteBufferPool.java b/core/src/main/java/io/undertow/server/DefaultByteBufferPool.java index f74a381bf6..519dd67d2d 100644 --- a/core/src/main/java/io/undertow/server/DefaultByteBufferPool.java +++ b/core/src/main/java/io/undertow/server/DefaultByteBufferPool.java @@ -26,7 +26,9 @@ import java.nio.ByteBuffer; import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; @@ -39,7 +41,7 @@ */ public class DefaultByteBufferPool implements ByteBufferPool { - private final ThreadLocal threadLocalCache = new ThreadLocal<>(); + private final ThreadLocalCache threadLocalCache = new ThreadLocalCache(); // Access requires synchronization on the threadLocalDataList instance private final List> threadLocalDataList = new ArrayList<>(); private final ConcurrentLinkedQueue queue = new ConcurrentLinkedQueue<>(); @@ -228,6 +230,7 @@ public void close() { local.buffers.clear(); } ref.clear(); + threadLocalCache.remove(local); } threadLocalDataList.clear(); } @@ -332,4 +335,29 @@ protected void finalize() throws Throwable { } } + // This is used instead of Java ThreadLocal class. Unlike in the ThreadLocal class, the remove() method in this + // class can be called by a different thread than the one that initialized the data. + private static class ThreadLocalCache { + + Map localsByThread = new HashMap<>(); + + ThreadLocalData get() { + return localsByThread.get(Thread.currentThread()); + } + + void set(ThreadLocalData threadLocalData) { + localsByThread.put(Thread.currentThread(), threadLocalData); + } + + void remove(ThreadLocalData threadLocalData) { + // Find the entry containing given data instance and remove it from the map. + for (Map.Entry entry: localsByThread.entrySet()) { + if (threadLocalData.equals(entry.getValue())) { + localsByThread.remove(entry.getKey(), entry.getValue()); + break; + } + } + } + } + } From fcef18b1019d4cc33684a2889416429e72d0adca Mon Sep 17 00:00:00 2001 From: mslovik Date: Mon, 23 Oct 2023 12:23:10 +0200 Subject: [PATCH 2/4] Add test case for UNDERTOW-2316 --- .../session/InMemorySessionTestCase.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/core/src/test/java/io/undertow/server/handlers/session/InMemorySessionTestCase.java b/core/src/test/java/io/undertow/server/handlers/session/InMemorySessionTestCase.java index eb29688ed8..b6d16448eb 100644 --- a/core/src/test/java/io/undertow/server/handlers/session/InMemorySessionTestCase.java +++ b/core/src/test/java/io/undertow/server/handlers/session/InMemorySessionTestCase.java @@ -20,6 +20,7 @@ import java.io.IOException; +import io.undertow.UndertowMessages; import io.undertow.server.HttpHandler; import io.undertow.server.HttpServerExchange; import io.undertow.server.session.InMemorySessionManager; @@ -249,4 +250,30 @@ public void run() { client.getConnectionManager().shutdown(); } } + + @Test + public void inMemorySessionNoConfigTest() throws IOException { + try (TestHttpClient client = new TestHttpClient()) { + client.setCookieStore(new BasicCookieStore()); + + final SessionAttachmentHandler handler = new SessionAttachmentHandler(new InMemorySessionManager(""), null); + handler.setNext(new HttpHandler() { + @Override + public void handleRequest(final HttpServerExchange exchange) throws Exception { + final SessionManager manager = exchange.getAttachment(SessionManager.ATTACHMENT_KEY); + + IllegalStateException thrown = Assert.assertThrows(IllegalStateException.class, () -> { + manager.getSession(exchange, null); + }); + + Assert.assertEquals(UndertowMessages.MESSAGES.couldNotFindSessionCookieConfig().getCause(), thrown.getCause()); + Assert.assertEquals(UndertowMessages.MESSAGES.couldNotFindSessionCookieConfig().getMessage(), thrown.getMessage()); + } + }); + DefaultServer.setRootHandler(handler); + + HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/notamatchingpath"); + client.execute(get); + } + } } From 30268f92219f4bac5facf6ca479503f80f48ca89 Mon Sep 17 00:00:00 2001 From: mslovik Date: Mon, 23 Oct 2023 12:23:10 +0200 Subject: [PATCH 3/4] Add test case for UNDERTOW-2316 --- .../session/InMemorySessionTestCase.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/core/src/test/java/io/undertow/server/handlers/session/InMemorySessionTestCase.java b/core/src/test/java/io/undertow/server/handlers/session/InMemorySessionTestCase.java index eb29688ed8..b6d16448eb 100644 --- a/core/src/test/java/io/undertow/server/handlers/session/InMemorySessionTestCase.java +++ b/core/src/test/java/io/undertow/server/handlers/session/InMemorySessionTestCase.java @@ -20,6 +20,7 @@ import java.io.IOException; +import io.undertow.UndertowMessages; import io.undertow.server.HttpHandler; import io.undertow.server.HttpServerExchange; import io.undertow.server.session.InMemorySessionManager; @@ -249,4 +250,30 @@ public void run() { client.getConnectionManager().shutdown(); } } + + @Test + public void inMemorySessionNoConfigTest() throws IOException { + try (TestHttpClient client = new TestHttpClient()) { + client.setCookieStore(new BasicCookieStore()); + + final SessionAttachmentHandler handler = new SessionAttachmentHandler(new InMemorySessionManager(""), null); + handler.setNext(new HttpHandler() { + @Override + public void handleRequest(final HttpServerExchange exchange) throws Exception { + final SessionManager manager = exchange.getAttachment(SessionManager.ATTACHMENT_KEY); + + IllegalStateException thrown = Assert.assertThrows(IllegalStateException.class, () -> { + manager.getSession(exchange, null); + }); + + Assert.assertEquals(UndertowMessages.MESSAGES.couldNotFindSessionCookieConfig().getCause(), thrown.getCause()); + Assert.assertEquals(UndertowMessages.MESSAGES.couldNotFindSessionCookieConfig().getMessage(), thrown.getMessage()); + } + }); + DefaultServer.setRootHandler(handler); + + HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/notamatchingpath"); + client.execute(get); + } + } } From 55fbacdd9f4508e701604fb1cd83d2ac92ba569f Mon Sep 17 00:00:00 2001 From: mslovik Date: Fri, 15 Dec 2023 12:27:46 +0100 Subject: [PATCH 4/4] Fix assertion scope --- .../session/InMemorySessionTestCase.java | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/core/src/test/java/io/undertow/server/handlers/session/InMemorySessionTestCase.java b/core/src/test/java/io/undertow/server/handlers/session/InMemorySessionTestCase.java index b6d16448eb..90f09d12a9 100644 --- a/core/src/test/java/io/undertow/server/handlers/session/InMemorySessionTestCase.java +++ b/core/src/test/java/io/undertow/server/handlers/session/InMemorySessionTestCase.java @@ -257,23 +257,30 @@ public void inMemorySessionNoConfigTest() throws IOException { client.setCookieStore(new BasicCookieStore()); final SessionAttachmentHandler handler = new SessionAttachmentHandler(new InMemorySessionManager(""), null); + final Throwable[] thrown = {null}; handler.setNext(new HttpHandler() { @Override public void handleRequest(final HttpServerExchange exchange) throws Exception { final SessionManager manager = exchange.getAttachment(SessionManager.ATTACHMENT_KEY); - - IllegalStateException thrown = Assert.assertThrows(IllegalStateException.class, () -> { + try { manager.getSession(exchange, null); - }); - - Assert.assertEquals(UndertowMessages.MESSAGES.couldNotFindSessionCookieConfig().getCause(), thrown.getCause()); - Assert.assertEquals(UndertowMessages.MESSAGES.couldNotFindSessionCookieConfig().getMessage(), thrown.getMessage()); + } catch (Throwable t) { + thrown[0] = t; + } } }); DefaultServer.setRootHandler(handler); HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/notamatchingpath"); client.execute(get); + + if (thrown[0] != null) { + Assert.assertTrue(thrown[0] instanceof IllegalStateException); + Assert.assertEquals(UndertowMessages.MESSAGES.couldNotFindSessionCookieConfig().getCause(), thrown[0].getCause()); + Assert.assertEquals(UndertowMessages.MESSAGES.couldNotFindSessionCookieConfig().getMessage(), thrown[0].getMessage()); + } else { + Assert.fail("No exception was thrown."); + } } } }