JAXBContext should be cached

[GoogleCodeExporter]

A new JAXBContext is created in BaseJAXBMarshaller and 
DefaultQualifyingPropertiesUnmarshaller whenever an instance is needed. This is 
bad because it is very expensive to create new instances, and the 
implementation generates some classes at runtime. When the code is run in JBoss 
4.2, this can lead to a memory leak due to caching of class information.

This was found in version 1.2.0, but the relevant code is unchanged in trunk.

Here is a patch that fixes this:

diff -ur 
../../src/XAdES4j/src/main/java/xades4j/xml/marshalling/BaseJAXBMarshaller.java 
./src/main/java/xades4j/xml/marshalling/BaseJAXBMarshaller.java
--- 
../../src/XAdES4j/src/main/java/xades4j/xml/marshalling/BaseJAXBMarshaller.java 
    2011-01-29 01:05:24.000000000 +0000
+++ ./src/main/java/xades4j/xml/marshalling/BaseJAXBMarshaller.java     
2012-06-18 16:49:18.346199259 +0000
@@ -40,6 +40,7 @@
 {
     private final Map<Class, QualifyingPropertyDataToXmlConverter<TXml>> converters;
     private final String propsElemName;
+    private static final Map<Class, JAXBContext> jaxbContexts = new 
HashMap<Class, JAXBContext>();

     protected BaseJAXBMarshaller(int convertersInitialSize, String propsElemName)
     {
@@ -140,8 +141,17 @@
     {
         try
         {
+            JAXBContext jaxbContext;
+            synchronized (jaxbContexts)
+            {
+                jaxbContext = jaxbContexts.get(xmlProps.getClass());
+                if (jaxbContext == null)
+                {
+                    jaxbContext = JAXBContext.newInstance(xmlProps.getClass());
+                    jaxbContexts.put(xmlProps.getClass(), jaxbContext);
+                }
+            }
             // Create the JAXB marshaller.
-            JAXBContext jaxbContext = 
JAXBContext.newInstance(xmlProps.getClass());
             Marshaller marshaller = jaxbContext.createMarshaller();
             // Create the root JAXBElement.
             Object propsElem = createPropsXmlElem(new ObjectFactory(), xmlProps);
diff -ur 
../../src/XAdES4j/src/main/java/xades4j/xml/unmarshalling/DefaultQualifyingPrope
rtiesUnmarshaller.java 
./src/main/java/xades4j/xml/unmarshalling/DefaultQualifyingPropertiesUnmarshalle
r.java
--- 
../../src/XAdES4j/src/main/java/xades4j/xml/unmarshalling/DefaultQualifyingPrope
rtiesUnmarshaller.java      2011-01-29 01:09:06.000000000 +0000
+++ 
./src/main/java/xades4j/xml/unmarshalling/DefaultQualifyingPropertiesUnmarshalle
r.java      2012-06-18 16:50:55.238196397 +0000
@@ -34,6 +34,7 @@
         implements QualifyingPropertiesUnmarshaller
 {
     private final UnmarshallerModule[] modules;
+    private static JAXBContext jaxbContext;

     public DefaultQualifyingPropertiesUnmarshaller()
     {
@@ -52,8 +53,11 @@
         XmlQualifyingPropertiesType xmlQualifyingProps = null;
         try
         {
-            // Create the JAXB unmarshaller.
-            JAXBContext jaxbContext = 
JAXBContext.newInstance(XmlQualifyingPropertiesType.class);
+            synchronized (DefaultQualifyingPropertiesUnmarshaller.class)
+            {
+                if (jaxbContext == null)
+                    jaxbContext = 
JAXBContext.newInstance(XmlQualifyingPropertiesType.class);
+            }
             // Create the JAXB unmarshaller and unmarshalProperties the root JAXB element
             Unmarshaller unmarshaller = jaxbContext.createUnmarshaller();
             JAXBElement<XmlQualifyingPropertiesType> qualifPropsElem = (JAXBElement<XmlQualifyingPropertiesType>)unmarshaller.unmarshal(qualifyingProps);

Original issue reported on code.google.com by petur...@gmail.com on 18 Jun 2012 at 5:17

[GoogleCodeExporter]

Will make this change as soon as possible. But I think the contexts could be 
final and initialized on the static constructors. Am I missing something?

Original comment by luis.fgoncalv on 18 Jun 2012 at 5:50

• Changed state: Accepted

[GoogleCodeExporter]

It would probably be cleaner to have the JAXBContext objects static final.
It was just done this way to make the smallest change possible since I
needed to fix it ASAP.

There are two main obstacles to declaring the objects static final: a) the
newInstance method throws a checked exception; and b) the parameter type
for BaseJAXBMarshaller depends on the subclass. They are both easy to
overcome, but require changing more code than this patch does.

Original comment by petur...@gmail.com on 18 Jun 2012 at 6:59

[GoogleCodeExporter]

Original comment by luis.fgoncalv on 22 Jun 2012 at 9:03

• Changed state: Fixed