From b8cc997cd03ce29b01f066b808d295c22807efe1 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sat, 11 Mar 2023 06:49:12 +0000 Subject: [PATCH] Bug 66521: Add a utility to clear all thread locals Otherwise some applications may complain about left-over things, e.g. Tomcat sometimes reports warning logs if Threads are not cleaned up before being passed back into the global thread-pool. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1908263 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/hslf/usermodel/HSLFSlideShow.java | 5 ++ .../poi/extractor/ExtractorFactory.java | 5 ++ .../record/crypto/Biff8EncryptionKey.java | 5 ++ .../poi/hssf/usermodel/HSSFCellStyle.java | 13 +++- .../org/apache/poi/sl/draw/DrawFactory.java | 5 ++ .../org/apache/poi/ss/usermodel/DateUtil.java | 9 +++ .../java/org/apache/poi/util/LocaleUtil.java | 7 +++ .../org/apache/poi/util/ThreadLocalUtil.java | 63 +++++++++++++++++++ .../apache/poi/util/TestThreadLocalUtil.java | 57 +++++++++++++++++ 9 files changed, 167 insertions(+), 2 deletions(-) create mode 100644 poi/src/main/java/org/apache/poi/util/ThreadLocalUtil.java create mode 100644 poi/src/test/java/org/apache/poi/util/TestThreadLocalUtil.java diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSlideShow.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSlideShow.java index 54438d7bb7..1447eab4b1 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSlideShow.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSlideShow.java @@ -66,6 +66,7 @@ import org.apache.poi.sl.usermodel.SlideShow; import org.apache.poi.util.GenericRecordUtil; import org.apache.poi.util.IOUtils; import org.apache.poi.util.Internal; +import org.apache.poi.util.ThreadLocalUtil; import org.apache.poi.util.Units; /** @@ -92,6 +93,10 @@ public final class HSLFSlideShow extends POIDocument implements SlideShow loadSavePhase = new ThreadLocal<>(); + static { + // allow to clear all thread-locals via ThreadLocalUtil + ThreadLocalUtil.registerCleaner(loadSavePhase::remove); + } // What we're based on private final HSLFSlideShowImpl _hslfSlideShow; diff --git a/poi/src/main/java/org/apache/poi/extractor/ExtractorFactory.java b/poi/src/main/java/org/apache/poi/extractor/ExtractorFactory.java index 3e8f046112..5c94c4c892 100644 --- a/poi/src/main/java/org/apache/poi/extractor/ExtractorFactory.java +++ b/poi/src/main/java/org/apache/poi/extractor/ExtractorFactory.java @@ -38,6 +38,7 @@ import org.apache.poi.poifs.filesystem.Entry; import org.apache.poi.poifs.filesystem.FileMagic; import org.apache.poi.poifs.filesystem.POIFSFileSystem; import org.apache.poi.util.IOUtils; +import org.apache.poi.util.ThreadLocalUtil; /** * Figures out the correct POIOLE2TextExtractor for your supplied @@ -64,6 +65,10 @@ public final class ExtractorFactory { /** Should this thread prefer event based over usermodel based extractors? */ private static final ThreadLocal threadPreferEventExtractors = ThreadLocal.withInitial(() -> Boolean.FALSE); + static { + // allow to clear all thread-locals via ThreadLocalUtil + ThreadLocalUtil.registerCleaner(threadPreferEventExtractors::remove); + } /** Should all threads prefer event based over usermodel based extractors? */ private static Boolean allPreferEventExtractors; diff --git a/poi/src/main/java/org/apache/poi/hssf/record/crypto/Biff8EncryptionKey.java b/poi/src/main/java/org/apache/poi/hssf/record/crypto/Biff8EncryptionKey.java index a96b80e37b..aa5efb29c8 100644 --- a/poi/src/main/java/org/apache/poi/hssf/record/crypto/Biff8EncryptionKey.java +++ b/poi/src/main/java/org/apache/poi/hssf/record/crypto/Biff8EncryptionKey.java @@ -17,6 +17,7 @@ package org.apache.poi.hssf.record.crypto; import org.apache.poi.hssf.usermodel.HSSFWorkbook; +import org.apache.poi.util.ThreadLocalUtil; public final class Biff8EncryptionKey { /** @@ -25,6 +26,10 @@ public final class Biff8EncryptionKey { * (e.g. {@link HSSFWorkbook}) that need this functionality. */ private static final ThreadLocal _userPasswordTLS = new ThreadLocal<>(); + static { + // allow to clear all thread-locals via ThreadLocalUtil + ThreadLocalUtil.registerCleaner(_userPasswordTLS::remove); + } /** * Sets the BIFF8 encryption/decryption password for the current thread. diff --git a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFCellStyle.java b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFCellStyle.java index 2dfaca3920..0983c45497 100644 --- a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFCellStyle.java +++ b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFCellStyle.java @@ -36,6 +36,7 @@ import org.apache.poi.ss.usermodel.Font; import org.apache.poi.ss.usermodel.HorizontalAlignment; import org.apache.poi.ss.usermodel.VerticalAlignment; import org.apache.poi.util.Removal; +import org.apache.poi.util.ThreadLocalUtil; /** * High level representation of the style of a cell in a sheet of a workbook. @@ -123,6 +124,14 @@ public final class HSSFCellStyle implements CellStyle, Duplicatable { private static final ThreadLocal lastDateFormat = ThreadLocal.withInitial(() -> Short.MIN_VALUE); private static final ThreadLocal> lastFormats = new ThreadLocal<>(); private static final ThreadLocal getDataFormatStringCache = new ThreadLocal<>(); + static { + // allow to clear all thread-locals via ThreadLocalUtil + ThreadLocalUtil.registerCleaner(() -> { + lastDateFormat.remove(); + lastFormats.remove(); + getDataFormatStringCache.remove(); + }); + } /** * Get the contents of the format string, by looking up @@ -637,7 +646,7 @@ public final class HSSFCellStyle implements CellStyle, Duplicatable { _format.setFillBackground(bg); checkDefaultBackgroundFills(); } - + /** * Set the background fill color represented as a {@link org.apache.poi.ss.usermodel.Color} value. *
@@ -696,7 +705,7 @@ public final class HSSFCellStyle implements CellStyle, Duplicatable { _format.setFillForeground(bg); checkDefaultBackgroundFills(); } - + /** * Set the foreground fill color represented as a {@link org.apache.poi.ss.usermodel.Color} value. *
diff --git a/poi/src/main/java/org/apache/poi/sl/draw/DrawFactory.java b/poi/src/main/java/org/apache/poi/sl/draw/DrawFactory.java index 6b63274455..9e17c867c8 100644 --- a/poi/src/main/java/org/apache/poi/sl/draw/DrawFactory.java +++ b/poi/src/main/java/org/apache/poi/sl/draw/DrawFactory.java @@ -38,9 +38,14 @@ import org.apache.poi.sl.usermodel.TableShape; import org.apache.poi.sl.usermodel.TextBox; import org.apache.poi.sl.usermodel.TextParagraph; import org.apache.poi.sl.usermodel.TextShape; +import org.apache.poi.util.ThreadLocalUtil; public class DrawFactory { private static final ThreadLocal defaultFactory = new ThreadLocal<>(); + static { + // allow to clear all thread-locals via ThreadLocalUtil + ThreadLocalUtil.registerCleaner(defaultFactory::remove); + } /** * Set a custom draw factory for the current thread. diff --git a/poi/src/main/java/org/apache/poi/ss/usermodel/DateUtil.java b/poi/src/main/java/org/apache/poi/ss/usermodel/DateUtil.java index 36f4af592e..bead9cef3c 100644 --- a/poi/src/main/java/org/apache/poi/ss/usermodel/DateUtil.java +++ b/poi/src/main/java/org/apache/poi/ss/usermodel/DateUtil.java @@ -36,6 +36,7 @@ import java.util.regex.Pattern; import org.apache.poi.ss.formula.ConditionalFormattingEvaluator; import org.apache.poi.util.LocaleUtil; +import org.apache.poi.util.ThreadLocalUtil; /** * Contains methods for dealing with Excel dates. @@ -546,6 +547,14 @@ public class DateUtil { private static final ThreadLocal lastFormatIndex = ThreadLocal.withInitial(() -> -1); private static final ThreadLocal lastFormatString = new ThreadLocal<>(); private static final ThreadLocal lastCachedResult = new ThreadLocal<>(); + static { + // allow to clear all thread-locals via ThreadLocalUtil + ThreadLocalUtil.registerCleaner(() -> { + lastFormatIndex.remove(); + lastFormatString.remove(); + lastCachedResult.remove(); + }); + } private static boolean isCached(String formatString, int formatIndex) { return formatIndex == lastFormatIndex.get() diff --git a/poi/src/main/java/org/apache/poi/util/LocaleUtil.java b/poi/src/main/java/org/apache/poi/util/LocaleUtil.java index 5f714522eb..1aca2e8fb9 100644 --- a/poi/src/main/java/org/apache/poi/util/LocaleUtil.java +++ b/poi/src/main/java/org/apache/poi/util/LocaleUtil.java @@ -57,6 +57,13 @@ public final class LocaleUtil { private static final ThreadLocal userTimeZone = new ThreadLocal<>(); private static final ThreadLocal userLocale = new ThreadLocal<>(); + static { + // allow to clear all thread-locals via ThreadLocalUtil + ThreadLocalUtil.registerCleaner(() -> { + userTimeZone.remove(); + userLocale.remove(); + }); + } /** * As time zone information is not stored in any format, it can be diff --git a/poi/src/main/java/org/apache/poi/util/ThreadLocalUtil.java b/poi/src/main/java/org/apache/poi/util/ThreadLocalUtil.java new file mode 100644 index 0000000000..2943caf646 --- /dev/null +++ b/poi/src/main/java/org/apache/poi/util/ThreadLocalUtil.java @@ -0,0 +1,63 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You 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 org.apache.poi.util; + +import java.util.ArrayList; +import java.util.List; + +/** + * Small utility to allow to remove references held in ThreadLocals. + * + * This is sometimes necessary, e.g. when returning threads into a global + * thread pool. + * + * For each usage of ThreadLocal, a cleaner is registered via + * registerCleaner(). + */ +public class ThreadLocalUtil { + private final static List registeredCleaners = new ArrayList<>(); + + private ThreadLocalUtil() { + } + + /** + * Clear {@link ThreadLocal}s of the current thread. + * + * This can be used to clean out a thread before "returning" + * it to a thread-pool or a Web-Container like Tomcat. + * + * Usually org.apache.xmlbeans.ThreadLocalUtil#clearAllThreadLocals() + * should be called as well to clear out some more ThreadLocals which + * are created by the XMLBeans library internally. + */ + public static void clearAllThreadLocals() { + // run all registered cleaners + registeredCleaners.forEach(Runnable::run); + } + + /** + * Intended for internal use only so other modules of Apache POi + * can add cleaners. + * + * @param cleaner a runnable which clears some thread-local that is + * located outside of the "poi" module. + */ + @Internal + public static void registerCleaner(Runnable cleaner) { + registeredCleaners.add(cleaner); + } +} diff --git a/poi/src/test/java/org/apache/poi/util/TestThreadLocalUtil.java b/poi/src/test/java/org/apache/poi/util/TestThreadLocalUtil.java new file mode 100644 index 0000000000..496040638c --- /dev/null +++ b/poi/src/test/java/org/apache/poi/util/TestThreadLocalUtil.java @@ -0,0 +1,57 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You 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 org.apache.poi.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.apache.poi.sl.draw.DrawFactory; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +public class TestThreadLocalUtil { + private final MemoryLeakVerifier verifier = new MemoryLeakVerifier(); + + @AfterEach + void tearDown() { + verifier.assertGarbageCollected(); + } + + @Test + public void testClearThreadLocalsNoData() { + // simply calling it without any thread locals should work + ThreadLocalUtil.clearAllThreadLocals(); + } + + @Test + public void testClearThreadLocalsWithData() { + DrawFactory factory = new DrawFactory(); + + // use the memory leak verifier to ensure that the thread-local is + // released after the clear-call below + verifier.addObject(factory); + + // store the object in a thread-local + DrawFactory.setDefaultFactory(factory); + + // retrieving it works now + assertEquals(factory, DrawFactory.getInstance(null)); + + // then clear them so that the verifier in tearDown() does not + // see the reference any longer + ThreadLocalUtil.clearAllThreadLocals(); + } +} \ No newline at end of file -- 2.39.5