Bug 563268 - Defer SAX Parser creation until required
The SAX parser factory is needed on demand to parse an XML file.
However, we only need the parser factory for the duration of the parse
and can release it afterwards. This speeds start-up, and can can allow
the SAX parser to be replaced dynamically between calls if needed.
Bug: 563268
Change-Id: I05896c0c1d0a671cc70e17b1651760c707e9e273
Signed-off-by: Alex Blewitt <alex.blewitt@gmail.com>
diff --git a/bundles/org.eclipse.core.contenttype/src/org/eclipse/core/internal/content/Activator.java b/bundles/org.eclipse.core.contenttype/src/org/eclipse/core/internal/content/Activator.java
index a855194..e38340b 100644
--- a/bundles/org.eclipse.core.contenttype/src/org/eclipse/core/internal/content/Activator.java
+++ b/bundles/org.eclipse.core.contenttype/src/org/eclipse/core/internal/content/Activator.java
@@ -14,7 +14,6 @@
package org.eclipse.core.internal.content;
import java.util.Hashtable;
-import javax.xml.parsers.SAXParserFactory;
import org.eclipse.core.runtime.IExtensionRegistry;
import org.eclipse.core.runtime.content.IContentTypeManager;
import org.eclipse.osgi.service.debug.DebugOptions;
@@ -30,7 +29,6 @@
private static Activator singleton;
private static BundleContext bundleContext;
private ServiceRegistration<IContentTypeManager> contentManagerService;
- private ServiceTracker<SAXParserFactory, Object> parserTracker;
private ServiceTracker<DebugOptions, Object> debugTracker;
private ServiceTracker<IExtensionRegistry, IExtensionRegistry> registryTracker;
@@ -56,10 +54,6 @@
contentManagerService.unregister();
contentManagerService = null;
}
- if (parserTracker != null) {
- parserTracker.close();
- parserTracker = null;
- }
if (debugTracker != null) {
debugTracker.close();
debugTracker = null;
@@ -73,21 +67,6 @@
}
/**
- * Return the registered SAX parser factory or null if one
- * does not exist.
- */
- public SAXParserFactory getFactory() {
- if (parserTracker == null) {
- parserTracker = new ServiceTracker<>(bundleContext, SAXParserFactory.class, null);
- parserTracker.open();
- }
- SAXParserFactory theFactory = (SAXParserFactory) parserTracker.getService();
- if (theFactory != null)
- theFactory.setNamespaceAware(true);
- return theFactory;
- }
-
- /**
* Return the boolean value in the debug options for the given key, or
* return the default value if there is none.
*/
diff --git a/bundles/org.eclipse.core.contenttype/src/org/eclipse/core/internal/content/XMLRootHandler.java b/bundles/org.eclipse.core.contenttype/src/org/eclipse/core/internal/content/XMLRootHandler.java
index daefa88..a7665e2 100644
--- a/bundles/org.eclipse.core.contenttype/src/org/eclipse/core/internal/content/XMLRootHandler.java
+++ b/bundles/org.eclipse.core.contenttype/src/org/eclipse/core/internal/content/XMLRootHandler.java
@@ -16,6 +16,7 @@
import java.io.IOException;
import java.io.StringReader;
import javax.xml.parsers.*;
+import org.osgi.framework.*;
import org.xml.sax.*;
import org.xml.sax.ext.LexicalHandler;
import org.xml.sax.helpers.DefaultHandler;
@@ -147,18 +148,33 @@
return namespaceFound;
}
+ private static final BundleContext CONTEXT;
+
+ static {
+ Bundle BUNDLE = FrameworkUtil.getBundle(XMLRootHandler.class);
+ CONTEXT = BUNDLE == null ? null : BUNDLE.getBundleContext();
+ }
+
public boolean parseContents(InputSource contents) throws IOException, ParserConfigurationException, SAXException {
// Parse the file into we have what we need (or an error occurs).
+ if (CONTEXT == null)
+ return false;
+ ServiceReference<SAXParserFactory> serviceReference = CONTEXT.getServiceReference(SAXParserFactory.class);
try {
- SAXParserFactory factory = Activator.getDefault().getFactory();
+
+ SAXParserFactory factory = CONTEXT.getService(serviceReference);
if (factory == null)
return false;
+ factory.setNamespaceAware(true);
final SAXParser parser = createParser(factory);
// to support external entities specified as relative URIs (see bug 63298)
contents.setSystemId("/"); //$NON-NLS-1$
parser.parse(contents, this);
+
} catch (StopParsingException e) {
// Abort the parsing normally. Fall through...
+ } finally {
+ CONTEXT.ungetService(serviceReference);
}
return true;
}
diff --git a/tests/org.eclipse.core.contenttype.tests/src/org/eclipse/core/internal/contenttype/tests/AllTests.java b/tests/org.eclipse.core.contenttype.tests/src/org/eclipse/core/internal/contenttype/tests/AllTests.java
index 73f3905..f4b3c7e 100644
--- a/tests/org.eclipse.core.contenttype.tests/src/org/eclipse/core/internal/contenttype/tests/AllTests.java
+++ b/tests/org.eclipse.core.contenttype.tests/src/org/eclipse/core/internal/contenttype/tests/AllTests.java
@@ -22,7 +22,7 @@
* eclipse.platform.resources repository (and move those tests here)
*/
@RunWith(Suite.class)
-@SuiteClasses(value = { UserContentTypeTest.class, })
+@SuiteClasses(value = { UserContentTypeTest.class, XMLRootHandlerTest.class, })
public class AllTests {
}
diff --git a/tests/org.eclipse.core.contenttype.tests/src/org/eclipse/core/internal/contenttype/tests/XMLRootHandlerTest.java b/tests/org.eclipse.core.contenttype.tests/src/org/eclipse/core/internal/contenttype/tests/XMLRootHandlerTest.java
new file mode 100644
index 0000000..941f3b6
--- /dev/null
+++ b/tests/org.eclipse.core.contenttype.tests/src/org/eclipse/core/internal/contenttype/tests/XMLRootHandlerTest.java
@@ -0,0 +1,53 @@
+/*******************************************************************************
+ * Copyright (c) 2020 Alex Blewitt and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * Alex Blewitt - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.core.internal.contenttype.tests;
+
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.io.StringReader;
+
+import javax.xml.parsers.ParserConfigurationException;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+
+import org.eclipse.core.internal.content.XMLRootHandler;
+
+import org.eclipse.core.runtime.CoreException;
+
+public class XMLRootHandlerTest {
+
+ private XMLRootHandler handler;
+
+ @Before
+ public void setUp() {
+ this.handler = new XMLRootHandler(true);
+ }
+
+ @After
+ public void tearDown() throws CoreException {
+ this.handler = null;
+ }
+
+ @Test
+ public void testParse() throws IOException, ParserConfigurationException, SAXException {
+ InputSource contents = new InputSource(new StringReader("<xml/>")); //$NON-NLS-1$
+
+ assertTrue(handler.parseContents(contents));
+ }
+}
