From 2b482dccf81f8a4ae726f82b79119d34cbb7263d Mon Sep 17 00:00:00 2001 From: vp340 Date: Tue, 29 Sep 2026 23:21:15 +0200 Subject: [PATCH 1/6] deregister from LoggingOutputStream the LoggingCallback after it has logged in order to avoid double log or memory leak when tmp file not deleted are involved (CXF-9251) --- .../apache/cxf/ext/logging/LoggingOutInterceptor.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java b/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java index 11a39cd2cf8..4efb260423a 100644 --- a/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java +++ b/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java @@ -214,6 +214,17 @@ public void onClose(CachedOutputStream cos) { // ignore } message.setContent(OutputStream.class, origStream); + + // The callback has done his job, so we release it. + // ------------- + // CXF-9251 + // If cos has a tmp file (so 'threshold' was triggered), after the introduction of DelayedCachedOutputStreamCleaner, a reference of LoggingOutputStream + // is held in a queue list ( DelayQueue queue ) + // If something goes wrong while closing the LoggingOutputStream, this can be recall and log twice (or more) when trying to delete the orphan tmp file. + // Furthermore, the LoggingOutputStream that registered this callback holds a reference to this Object, so it remains in memory avoiding GC until the DelayedCachedOutputStreamCleaner does his job (default 30 minutes) + // ------------- + // Doing, instead, this we should be ok :) + cos.deregisterCallback(this); } private void copyPayload(CachedOutputStream cos, final LogEvent event) { From f1005ed4f0702227509b1387f2153b4cfa1ff180 Mon Sep 17 00:00:00 2001 From: vp340 Date: Wed, 30 Sep 2026 00:07:35 +0200 Subject: [PATCH 2/6] fix comment --- .../java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java b/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java index 4efb260423a..7b104cf0641 100644 --- a/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java +++ b/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java @@ -223,7 +223,7 @@ public void onClose(CachedOutputStream cos) { // If something goes wrong while closing the LoggingOutputStream, this can be recall and log twice (or more) when trying to delete the orphan tmp file. // Furthermore, the LoggingOutputStream that registered this callback holds a reference to this Object, so it remains in memory avoiding GC until the DelayedCachedOutputStreamCleaner does his job (default 30 minutes) // ------------- - // Doing, instead, this we should be ok :) + // Doing this, instead, we should be ok :) cos.deregisterCallback(this); } From b02828100db84c2a5c74bd82a085b9aa272627cb Mon Sep 17 00:00:00 2001 From: vp340 Date: Wed, 30 Sep 2026 17:30:17 +0200 Subject: [PATCH 3/6] remove the cos.deregisterCallback(this); ... onClose is inside a loop so it launched : java.util.ConcurrentModificationException --- .../apache/cxf/ext/logging/LoggingOutInterceptor.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java b/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java index 7b104cf0641..11a39cd2cf8 100644 --- a/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java +++ b/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java @@ -214,17 +214,6 @@ public void onClose(CachedOutputStream cos) { // ignore } message.setContent(OutputStream.class, origStream); - - // The callback has done his job, so we release it. - // ------------- - // CXF-9251 - // If cos has a tmp file (so 'threshold' was triggered), after the introduction of DelayedCachedOutputStreamCleaner, a reference of LoggingOutputStream - // is held in a queue list ( DelayQueue queue ) - // If something goes wrong while closing the LoggingOutputStream, this can be recall and log twice (or more) when trying to delete the orphan tmp file. - // Furthermore, the LoggingOutputStream that registered this callback holds a reference to this Object, so it remains in memory avoiding GC until the DelayedCachedOutputStreamCleaner does his job (default 30 minutes) - // ------------- - // Doing this, instead, we should be ok :) - cos.deregisterCallback(this); } private void copyPayload(CachedOutputStream cos, final LogEvent event) { From 2ada67995df4c32b969e257cb0f70eb70c9b428b Mon Sep 17 00:00:00 2001 From: vp340 Date: Wed, 30 Sep 2026 17:49:34 +0200 Subject: [PATCH 4/6] implement a new simple OneTimeLoggingCallback that wraps the original LoggingCallback, ensuring log only once, and prevent memory leaks --- .../ext/logging/LoggingOutInterceptor.java | 49 ++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java b/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java index 11a39cd2cf8..745a109263e 100644 --- a/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java +++ b/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java @@ -70,7 +70,8 @@ public void handleMessage(Message message) throws Fault { createExchangeId(message); final OutputStream os = message.getContent(OutputStream.class); if (os != null) { - LoggingCallback callback = new LoggingCallback(sender, message, os, limit); + // Wrap the callback to ensure logging only once and to avoid memory leaks (CXF-9251) + OneTimeLoggingCallback callback = new OneTimeLoggingCallback(new LoggingCallback(sender, message, os, limit)); message.setContent(OutputStream.class, createCachingOut(message, os, callback)); } else { final Writer iowriter = message.getContent(Writer.class); @@ -179,6 +180,52 @@ private void writePayload(StringBuilder builder, StringWriter stringWriter, LogE } } + + /*** + * [CXF-9251] + * If CachedOutputStream cos has a tmp file (so 'threshold' was triggered), after the introduction of + * DelayedCachedOutputStreamCleaner, a reference of LoggingOutputStream + * is held in a queue list ( DelayQueue queue ) + * If something goes wrong while closing the LoggingOutputStream, onClose() can be recall + * and log twice (or more) when trying to delete the orphan tmp file. + * Furthermore, the LoggingOutputStream that registered LoggingCallback holds a reference to that + * Object (and its attributes... like Message instance), so it remains in memory avoiding GC until the + * DelayedCachedOutputStreamCleaner does his job (default 30 minutes) + * ----------- + * Solution: + * This class ensures we log only once, and then dereferences the original LoggingCallback, making it + * eligible for garbage collection. + */ + public class OneTimeLoggingCallback implements CachedOutputStreamCallback { + private LoggingCallback wrappedCallback; + + public OneTimeLoggingCallback(LoggingCallback wrappedCallback) { + this.wrappedCallback = wrappedCallback; + } + + @Override + public void onClose(CachedOutputStream cos) { + // Ensure to log only once + if (wrappedCallback != null) { + try { + wrappedCallback.onClose(cos); + } finally { + // Make the original Callback (and its attribute) eligible for GC + // This is especially useful if the cos with a reference for this Callback + // is still held in a list (such as the one used by DelayedCachedOutputStreamCleaner) + this.wrappedCallback = null; + } + } + } + + @Override + public void onFlush(CachedOutputStream cos) { + if (wrappedCallback != null) { + wrappedCallback.onFlush(cos); + } + } + } + public class LoggingCallback implements CachedOutputStreamCallback { private final Message message; From 59d78e010a002ce05eccbfa7719b492714f9867a Mon Sep 17 00:00:00 2001 From: vp340 Date: Wed, 30 Sep 2026 18:08:19 +0200 Subject: [PATCH 5/6] indentation --- .../org/apache/cxf/ext/logging/LoggingOutInterceptor.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java b/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java index 745a109263e..1799586d9bf 100644 --- a/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java +++ b/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java @@ -71,7 +71,9 @@ public void handleMessage(Message message) throws Fault { final OutputStream os = message.getContent(OutputStream.class); if (os != null) { // Wrap the callback to ensure logging only once and to avoid memory leaks (CXF-9251) - OneTimeLoggingCallback callback = new OneTimeLoggingCallback(new LoggingCallback(sender, message, os, limit)); + OneTimeLoggingCallback callback = new OneTimeLoggingCallback( + new LoggingCallback(sender, message, os, limit) + ); message.setContent(OutputStream.class, createCachingOut(message, os, callback)); } else { final Writer iowriter = message.getContent(Writer.class); From a02cd8ac5b9c0d1bac41f51124d96ae47bcbafe7 Mon Sep 17 00:00:00 2001 From: vp340 Date: Fri, 2 Oct 2026 17:47:55 +0200 Subject: [PATCH 6/6] Due to multithread nature of the bug, I think it's necessary to make sure the logging doesn't happen twice using also an atomic boolean. (if clean is set to 2 seconds and this.wrappedCallback = null is only set in the cpu cache and not wrote in memory) --- .../org/apache/cxf/ext/logging/LoggingOutInterceptor.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java b/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java index 1799586d9bf..9aa8a3cba85 100644 --- a/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java +++ b/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java @@ -24,6 +24,7 @@ import java.io.PrintWriter; import java.io.StringWriter; import java.io.Writer; +import java.util.concurrent.atomic.AtomicBoolean; import org.apache.cxf.common.injection.NoJSR250Annotations; import org.apache.cxf.common.util.StringUtils; @@ -200,6 +201,7 @@ private void writePayload(StringBuilder builder, StringWriter stringWriter, LogE */ public class OneTimeLoggingCallback implements CachedOutputStreamCallback { private LoggingCallback wrappedCallback; + private final AtomicBoolean alreadyClosed = new AtomicBoolean(false); public OneTimeLoggingCallback(LoggingCallback wrappedCallback) { this.wrappedCallback = wrappedCallback; @@ -208,7 +210,7 @@ public OneTimeLoggingCallback(LoggingCallback wrappedCallback) { @Override public void onClose(CachedOutputStream cos) { // Ensure to log only once - if (wrappedCallback != null) { + if (alreadyClosed.compareAndSet(false, true) && wrappedCallback != null) { try { wrappedCallback.onClose(cos); } finally { @@ -222,7 +224,7 @@ public void onClose(CachedOutputStream cos) { @Override public void onFlush(CachedOutputStream cos) { - if (wrappedCallback != null) { + if (!alreadyClosed.get() && wrappedCallback != null) { wrappedCallback.onFlush(cos); } }