You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@santuario.apache.org by co...@apache.org on 2018/05/22 10:44:25 UTC
svn commit: r1832014 - in /santuario/xml-security-java/trunk/src:
main/java/org/apache/xml/security/utils/XMLUtils.java
test/java/org/apache/xml/security/test/dom/utils/DocumentBuilderPoolingTest.java
Author: coheigea
Date: Tue May 22 10:44:25 2018
New Revision: 1832014
URL: http://svn.apache.org/viewvc?rev=1832014&view=rev
Log:
Revert "Switch to using ThreadLocals instead of a a Thread pool in XMLUtils"
This reverts commit 9fd55fca4b884f34ca5cd7d8c338dbec8806a75b.
Added:
santuario/xml-security-java/trunk/src/test/java/org/apache/xml/security/test/dom/utils/DocumentBuilderPoolingTest.java
Modified:
santuario/xml-security-java/trunk/src/main/java/org/apache/xml/security/utils/XMLUtils.java
Modified: santuario/xml-security-java/trunk/src/main/java/org/apache/xml/security/utils/XMLUtils.java
URL: http://svn.apache.org/viewvc/santuario/xml-security-java/trunk/src/main/java/org/apache/xml/security/utils/XMLUtils.java?rev=1832014&r1=1832013&r2=1832014&view=diff
==============================================================================
--- santuario/xml-security-java/trunk/src/main/java/org/apache/xml/security/utils/XMLUtils.java (original)
+++ santuario/xml-security-java/trunk/src/main/java/org/apache/xml/security/utils/XMLUtils.java Tue May 22 10:44:25 2018
@@ -18,7 +18,9 @@
*/
package org.apache.xml.security.utils;
+import java.io.File;
import java.io.IOException;
+import java.io.InputStream;
import java.io.OutputStream;
import java.math.BigInteger;
import java.security.AccessController;
@@ -34,17 +36,23 @@ import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.validation.Schema;
import org.apache.xml.security.c14n.CanonicalizationException;
import org.apache.xml.security.c14n.Canonicalizer;
import org.apache.xml.security.c14n.InvalidCanonicalizerException;
import org.w3c.dom.Attr;
+import org.w3c.dom.DOMImplementation;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.NamedNodeMap;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import org.w3c.dom.Text;
+import org.xml.sax.EntityResolver;
+import org.xml.sax.ErrorHandler;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
/**
* DOM and XML accessibility and comfort functions.
@@ -57,12 +65,12 @@ public final class XMLUtils {
(PrivilegedAction<Boolean>) () -> Boolean.getBoolean("org.apache.xml.security.ignoreLineBreaks"));
@SuppressWarnings("unchecked")
- private static final ThreadLocal<DocumentBuilder> tl[][] = new ThreadLocal[2][2];
+ private static final WeakObjectPool<DocumentBuilder, ParserConfigurationException> pools[] = new WeakObjectPool[4];
static {
- tl[0][0] = new MyThreadLocal(false, false);
- tl[0][1] = new MyThreadLocal(false, true);
- tl[1][0] = new MyThreadLocal(true, false);
- tl[1][1] = new MyThreadLocal(true, true);
+ pools[0] = new DocumentBuilderPool(false, false);
+ pools[1] = new DocumentBuilderPool(false, true);
+ pools[2] = new DocumentBuilderPool(true, false);
+ pools[3] = new DocumentBuilderPool(true, true);
}
private static volatile String dsPrefix = "ds";
@@ -1061,14 +1069,25 @@ public final class XMLUtils {
public static DocumentBuilder createDocumentBuilder(
boolean validating, boolean disAllowDocTypeDeclarations
) throws ParserConfigurationException {
- DocumentBuilder documentBuilder = tl[validating ? 1 : 0][disAllowDocTypeDeclarations ? 1 : 0].get();
- documentBuilder.reset();
- return documentBuilder;
+ int idx = getPoolsIndex(validating, disAllowDocTypeDeclarations);
+ return pools[idx].getObject();
}
- @Deprecated
+
+ /**
+ * Return this document builder to be reused
+ * @param db DocumentBuilder returned from any of {@link #createDocumentBuilder} methods.
+ * @return whether it was successfully returned to the pool
+ */
public static boolean repoolDocumentBuilder(DocumentBuilder db) {
- return true;
+ if (!(db instanceof DocumentBuilderProxy)) {
+ return false;
+ }
+ db.reset();
+ boolean disAllowDocTypeDeclarations =
+ ((DocumentBuilderProxy)db).disAllowDocTypeDeclarations();
+ int idx = getPoolsIndex(db.isValidating(), disAllowDocTypeDeclarations);
+ return pools[idx].repool(db);
}
/**
@@ -1116,30 +1135,121 @@ public final class XMLUtils {
return resizedBytes;
}
- private static final class MyThreadLocal extends ThreadLocal<DocumentBuilder> {
+ /**
+ * We need this proxy wrapping DocumentBuilder to record the value
+ * passed to disAllowDoctypeDeclarations. It's needed to figure out
+ * on which pool to return.
+ */
+ private static class DocumentBuilderProxy extends DocumentBuilder {
+ private final DocumentBuilder delegate;
+ private final boolean disAllowDocTypeDeclarations;
+
+ private DocumentBuilderProxy(DocumentBuilder actual, boolean disAllowDocTypeDeclarations) {
+ delegate = actual;
+ this.disAllowDocTypeDeclarations = disAllowDocTypeDeclarations;
+ }
+
+ boolean disAllowDocTypeDeclarations() {
+ return disAllowDocTypeDeclarations;
+ }
+
+ public void reset() {
+ delegate.reset();
+ }
+
+ public Document parse(InputStream is) throws SAXException, IOException {
+ return delegate.parse(is);
+ }
+
+ public Document parse(InputStream is, String systemId)
+ throws SAXException, IOException {
+ return delegate.parse(is, systemId);
+ }
+
+ public Document parse(String uri) throws SAXException, IOException {
+ return delegate.parse(uri);
+ }
+
+ public Document parse(File f) throws SAXException, IOException {
+ return delegate.parse(f);
+ }
+
+ public Schema getSchema() {
+ return delegate.getSchema();
+ }
+
+ public boolean isXIncludeAware() {
+ return delegate.isXIncludeAware();
+ }
+
+ @Override
+ public Document parse(InputSource is) throws SAXException, IOException {
+ return delegate.parse(is);
+ }
+
+ @Override
+ public boolean isNamespaceAware() {
+ return delegate.isNamespaceAware();
+ }
+
+ @Override
+ public boolean isValidating() {
+ return delegate.isValidating();
+ }
+
+ @Override
+ public void setEntityResolver(EntityResolver er) {
+ delegate.setEntityResolver(er);
+ }
+
+ @Override
+ public void setErrorHandler(ErrorHandler eh) {
+ delegate.setErrorHandler(eh);
+ }
+
+ @Override
+ public Document newDocument() {
+ return delegate.newDocument();
+ }
+
+ @Override
+ public DOMImplementation getDOMImplementation() {
+ return delegate.getDOMImplementation();
+ }
+
+ }
+
+ private static final class DocumentBuilderPool
+ extends WeakObjectPool<DocumentBuilder, ParserConfigurationException> {
private final boolean validating, disAllowDocTypeDeclarations;
- public MyThreadLocal(boolean validating, boolean disAllowDocTypeDeclarations) {
+ public DocumentBuilderPool(boolean validating, boolean disAllowDocTypeDeclarations) {
this.validating = validating;
this.disAllowDocTypeDeclarations = disAllowDocTypeDeclarations;
}
@Override
- protected DocumentBuilder initialValue() {
- try {
- DocumentBuilderFactory dfactory = DocumentBuilderFactory.newInstance();
- dfactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, Boolean.TRUE);
- if (disAllowDocTypeDeclarations) {
- dfactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
- }
- dfactory.setValidating(validating);
- dfactory.setNamespaceAware(true);
- return dfactory.newDocumentBuilder();
- } catch (ParserConfigurationException e) {
- throw new RuntimeException(e);
+ protected DocumentBuilder createObject() throws ParserConfigurationException {
+ DocumentBuilderFactory dfactory = DocumentBuilderFactory.newInstance();
+ dfactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, Boolean.TRUE);
+ if (disAllowDocTypeDeclarations) {
+ dfactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
}
+ dfactory.setValidating(validating);
+ dfactory.setNamespaceAware(true);
+ return new DocumentBuilderProxy(dfactory.newDocumentBuilder(), disAllowDocTypeDeclarations);
}
}
+ /**
+ * Maps the two boolean configuration options for the factories to the array index for the WeakObjectPool
+ * @param validating
+ * @param disAllowDocTypeDeclarations
+ * @return the index to the {@link #pools}
+ */
+ private static int getPoolsIndex(boolean validating, boolean disAllowDocTypeDeclarations) {
+ return (validating ? 2 : 0) + (disAllowDocTypeDeclarations ? 1 : 0);
+ }
+
}
Added: santuario/xml-security-java/trunk/src/test/java/org/apache/xml/security/test/dom/utils/DocumentBuilderPoolingTest.java
URL: http://svn.apache.org/viewvc/santuario/xml-security-java/trunk/src/test/java/org/apache/xml/security/test/dom/utils/DocumentBuilderPoolingTest.java?rev=1832014&view=auto
==============================================================================
--- santuario/xml-security-java/trunk/src/test/java/org/apache/xml/security/test/dom/utils/DocumentBuilderPoolingTest.java (added)
+++ santuario/xml-security-java/trunk/src/test/java/org/apache/xml/security/test/dom/utils/DocumentBuilderPoolingTest.java Tue May 22 10:44:25 2018
@@ -0,0 +1,175 @@
+/**
+ * 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.xml.security.test.dom.utils;
+
+import org.apache.xml.security.utils.WeakObjectPool;
+import org.apache.xml.security.utils.XMLUtils;
+import org.junit.Test;
+
+import javax.xml.parsers.DocumentBuilder;
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Field;
+import java.util.Iterator;
+import java.util.concurrent.*;
+
+import static org.junit.Assert.*;
+
+public class DocumentBuilderPoolingTest {
+
+ private static final String DOCUMENTBUILDERPROXY_CLASSNAME =
+ "org.apache.xml.security.utils.XMLUtils$DocumentBuilderProxy";
+
+ @Test
+ public void testEquals() throws Exception {
+ DocumentBuilder documentBuilder = XMLUtils.createDocumentBuilder(true);
+ assertEquals(documentBuilder, documentBuilder);
+ }
+
+ @Test
+ public void testGetValidatingDocumentBuilder() throws Exception {
+ DocumentBuilder documentBuilder = XMLUtils.createDocumentBuilder(true);
+ assertTrue(documentBuilder.isValidating());
+ }
+
+ @Test
+ public void testGetNonValidatingDocumentBuilder() throws Exception {
+ DocumentBuilder documentBuilder = XMLUtils.createDocumentBuilder(false);
+ assertFalse(documentBuilder.isValidating());
+ }
+
+ @Test
+ public void testGetValidatingAndAllowDocTypeDeclarationsDocumentBuilder() throws Exception {
+ DocumentBuilder documentBuilder = XMLUtils.createDocumentBuilder(true, false);
+ assertTrue(documentBuilder.isValidating());
+ assertEquals(documentBuilder.getClass().getName(), DOCUMENTBUILDERPROXY_CLASSNAME);
+ assertAllowDocTypeDeclarations(documentBuilder, false);
+ }
+
+ @Test
+ public void testGetValidatingAndDisAllowDocTypeDeclarationsDocumentBuilder() throws Exception {
+ DocumentBuilder documentBuilder = XMLUtils.createDocumentBuilder(true, true);
+ assertTrue(documentBuilder.isValidating());
+ assertEquals(documentBuilder.getClass().getName(), DOCUMENTBUILDERPROXY_CLASSNAME);
+ assertAllowDocTypeDeclarations(documentBuilder, true);
+ }
+
+ private void assertAllowDocTypeDeclarations(DocumentBuilder documentBuilder, boolean allow) throws Exception {
+ Field field = documentBuilder.getClass().getDeclaredField("disAllowDocTypeDeclarations");
+ field.setAccessible(true);
+ assertEquals(allow, field.get(documentBuilder));
+ }
+
+ @Test
+ public void testNewDocumentBuilderInstances() throws Exception {
+ int count = 4;
+
+ // get all possible combinations of DocumentBuilders:
+ DocumentBuilder[] documentBuilders = new DocumentBuilder[count];
+ for (int i = 0; i < count; i++) {
+ documentBuilders[i] = XMLUtils.createDocumentBuilder(i / 2 > 0, i % 2 == 1);
+ }
+
+ //test that we got always a new instance:
+ for (int i = 0; i < count; i++) {
+ for (int j = i + 1; j < count; j++) {
+ assertNotEquals(documentBuilders[i], documentBuilders[j]);
+ assertNotSame(documentBuilders[i], documentBuilders[j]);
+ }
+ }
+ }
+
+ @Test
+ public void testRepoolingTwice() throws Exception {
+ DocumentBuilder documentBuilder = XMLUtils.createDocumentBuilder(true);
+ assertTrue(XMLUtils.repoolDocumentBuilder(documentBuilder));
+ assertFalse("can't repool the same object twice!", XMLUtils.repoolDocumentBuilder(documentBuilder));
+ }
+
+ @Test(timeout = 30000)
+ public void testPooling() throws Exception {
+ int nThreads = 8;
+ ExecutorService exec = Executors.newFixedThreadPool(nThreads);
+ Future<?>[] results = new Future[nThreads];
+ for (int i = 0; i < nThreads - 1; i++) {
+ results[i] = exec.submit(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ while (true) {
+ // retrieve some DocumentBuilders...
+ DocumentBuilder documentBuilders[] = new DocumentBuilder[10];
+ for (int j = 0; j < documentBuilders.length; j++) {
+ documentBuilders[j] = XMLUtils.createDocumentBuilder(false);
+ assertNotNull(documentBuilders[j]);
+ }
+ // ...then repool them so that another thread may pickup them again
+ for (int j = 0; j < documentBuilders.length; j++) {
+ assertTrue(XMLUtils.repoolDocumentBuilder(documentBuilders[j]));
+ }
+ }
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+ });
+ }
+ // more or less mimic gc
+ results[nThreads - 1] = exec.submit(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ final Field poolField = XMLUtils.class.getDeclaredField("pools");
+ poolField.setAccessible(true);
+ final WeakObjectPool[] weakObjectPools = (WeakObjectPool[]) poolField.get(null);
+
+ final Field availableField = WeakObjectPool.class.getDeclaredField("available");
+ availableField.setAccessible(true);
+
+ while (true) {
+ final BlockingDeque blockingDeque = (BlockingDeque) availableField.get(weakObjectPools[1]);
+ Iterator iterator = blockingDeque.iterator();
+ while (iterator.hasNext()) {
+ ((WeakReference) iterator.next()).clear();
+ }
+ Thread.sleep(200);
+ }
+ } catch (InterruptedException e) {
+ return;
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+ });
+
+ exec.shutdown();
+ exec.awaitTermination(5, TimeUnit.SECONDS);
+ for (Future<?> f : results) {
+ if (!f.isDone()) {
+ f.cancel(false);
+ }
+ try {
+ assertNull(f.get(1000, TimeUnit.MILLISECONDS));
+ } catch (CancellationException ce) {
+ //expected since we did cancel it
+ } catch (TimeoutException e) {
+ fail(f + "didn't cancel after timeout?");
+ }
+ }
+ }
+}