-
Notifications
You must be signed in to change notification settings - Fork 664
Description
Describe the bug
I identified a structural defect (UTL-14) in org.apache.karaf.util.XmlUtils. The class uses static ThreadLocal variables to cache XML factory instances (DocumentBuilderFactory, TransformerFactory, SAXParserFactory) without any cleanup mechanism (remove() is never called).
In an OSGi environment (Karaf), these factory instances often originate from specific bundles (e.g., Xerces or system bundle fragments). Holding them in static ThreadLocals of long-lived Karaf worker threads pins the ClassLoader of the provider bundle.
Source Code Location:
// org/apache/karaf/util/XmlUtils.java (Lines 51-53)
private static final ThreadLocal DOCUMENT_BUILDER_FACTORY = new ThreadLocal<>();
private static final ThreadLocal TRANSFORMER_FACTORY = new ThreadLocal<>();
private static final ThreadLocal SAX_PARSER_FACTORY = new ThreadLocal<>();
// Usage example (Line 132):
// .set() is called, but .remove() is never called anywhere in the class.
TRANSFORMER_FACTORY.set(tf);
Impact
-
OSGi Bundle Locking (Zombie Bundles): If the bundle providing the XML implementation is updated or refreshed, the old ClassLoader cannot be garbage collected because it is strongly referenced by the ThreadLocalMap of active threads.
-
Metaspace Leak: Repeated updates of XML-related bundles will lead to permanent Metaspace growth.
-
Stale Logic: Threads may continue to use XML factories from an uninstalled/obsolete bundle version.
Expected behavior
XmlUtils should avoid using static ThreadLocal for holding OSGi-dependent resources, or ensure that remove() is called after usage. Given that XmlUtils is a static utility class with no lifecycle management, removing the ThreadLocal caching (instantiating factories per request) or using a SoftReference approach is recommended to ensure OSGi stability.
Actual behavior
The XML factories are cached indefinitely on threads, preventing the underlying provider bundles from being fully unloaded/refreshed.
Suggested Fix
Remove the static ThreadLocal caching entirely. Although creating XML factories has a small performance cost, correct ClassLoader handling in OSGi is critical.
Proposed Change:
// Remove static ThreadLocals
// private static final ThreadLocal DOCUMENT_BUILDER_FACTORY = new ThreadLocal<>();
// ...
public static DocumentBuilder documentBuilder() throws ParserConfigurationException {
// Instead of retrieving from ThreadLocal, create a new instance every time
// to ensure the correct ClassLoader (from the current context/bundle) is used.
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setNamespaceAware(true);
// ... set other features ...
return dbf.newDocumentBuilder();
}
Alternatively, if performance is a blocker, use a SoftReference or implement a Closeable hook to clear ThreadLocals on bundle stop, but removing the cache is the safest for static utility classes.