Use a ThreadLocal XPathFactory

XPathFactory is documented to be non-threadsafe. While we could use
single shared instance, accessing it would require synchronization. Use
a ThreadLocal, so each thread gets its own instance.

Change-Id: I3f6a722df94d261c0680b30b9b65b73566350794
Signed-off-by: Robert Varga <rovarga@cisco.com>

diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/Utils.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/Utils.java
index 389456e361f7b4a7efd603c63325dc0a65ded59e..5e56fecc7edf4efdc5fab31d23b386712af7d618 100644 (file)
--- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/Utils.java
+++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/Utils.java
@@ -25,8 +25,8 @@ import java.util.Objects;
 import java.util.Set;
 import java.util.regex.Pattern;
 import javax.annotation.Nullable;
-import javax.annotation.concurrent.GuardedBy;
 import javax.xml.xpath.XPath;
+import javax.xml.xpath.XPathExpression;
 import javax.xml.xpath.XPathExpressionException;
 import javax.xml.xpath.XPathFactory;
 import org.antlr.v4.runtime.tree.TerminalNode;
@@ -74,14 +74,25 @@ public final class Utils {
     private static final Splitter SPACE_SPLITTER = Splitter.on(' ').omitEmptyStrings().trimResults();
     private static final Pattern PATH_ABS = Pattern.compile("/[^/].*");
 
-    // XPathFactory is documented as non-threadsafe
-    @GuardedBy("Utils.class")
-    private static final XPathFactory XPATH_FACTORY = XPathFactory.newInstance();
+    private static final ThreadLocal<XPathFactory> XPATH_FACTORY = new ThreadLocal<XPathFactory>() {
+        @Override
+        protected XPathFactory initialValue() {
+            return XPathFactory.newInstance();
+        }
+    };
 
     private Utils() {
         throw new UnsupportedOperationException();
     }
 
+    /**
+     * Cleanup any resources attached to the current thread. Threads interacting with this class can cause thread-local
+     * caches to them. Invoke this method if you want to detach those resources.
+     */
+    public static void detachFromCurrentThread() {
+        XPATH_FACTORY.remove();
+    }
+
     public static Collection<SchemaNodeIdentifier.Relative> transformKeysStringToKeyNodes(final StmtContext<?, ?, ?> ctx,
             final String value) {
         List<String> keyTokens = SPACE_SPLITTER.splitToList(value);
@@ -108,11 +119,8 @@ public final class Utils {
         return SLASH_SPLITTER.splitToList(path);
     }
 
-    public static void validateXPath(final StmtContext<?, ?, ?> ctx, final String path) {
-        final XPath xPath;
-        synchronized (Utils.class) {
-            xPath = XPATH_FACTORY.newXPath();
-        }
+    private static void compileXPath(final StmtContext<?, ?, ?> ctx, final String path) {
+        final XPath xPath = XPATH_FACTORY.get().newXPath();
 
         try {
             xPath.compile(path);
@@ -126,8 +134,9 @@ public final class Utils {
     }
 
     public static boolean isXPathAbsolute(final StmtContext<?, ?, ?> ctx, final String path) {
-
-        validateXPath(ctx, trimSingleLastSlashFromXPath(path));
+        // FIXME: this is probably an overkill, as this is called for all XPath objects. If we need this validation,
+        //        we should wrap the resulting XPath into RevisionAwareXPath.
+        compileXPath(ctx, trimSingleLastSlashFromXPath(path));
 
         return PATH_ABS.matcher(path).matches();
     }
@@ -180,7 +189,7 @@ public final class Utils {
 
         String trimmedPath = trimSingleLastSlashFromXPath(path);
 
-        validateXPath(ctx, trimmedPath);
+        compileXPath(ctx, trimmedPath);
 
         List<String> nodeNames = splitPathToNodeNames(trimmedPath);
         List<QName> qNames = new ArrayList<>(nodeNames.size());