You are viewing a plain text version of this content. The canonical link for it is here.
Posted to java-dev@axis.apache.org by sc...@apache.org on 2007/10/25 19:37:28 UTC

svn commit: r588288 - in /webservices/axis2/trunk/java/modules/jaxws: src/org/apache/axis2/jaxws/message/databinding/JAXBUtils.java test/org/apache/axis2/jaxws/misc/JAXBContextTest.java

Author: scheu
Date: Thu Oct 25 10:37:27 2007
New Revision: 588288

URL: http://svn.apache.org/viewvc?rev=588288&view=rev
Log:
AXIS2:3302
Contributor:Rich Scheuerle
Remove unnecessary synchronization in JAXBUtils.

Modified:
    webservices/axis2/trunk/java/modules/jaxws/src/org/apache/axis2/jaxws/message/databinding/JAXBUtils.java
    webservices/axis2/trunk/java/modules/jaxws/test/org/apache/axis2/jaxws/misc/JAXBContextTest.java

Modified: webservices/axis2/trunk/java/modules/jaxws/src/org/apache/axis2/jaxws/message/databinding/JAXBUtils.java
URL: http://svn.apache.org/viewvc/webservices/axis2/trunk/java/modules/jaxws/src/org/apache/axis2/jaxws/message/databinding/JAXBUtils.java?rev=588288&r1=588287&r2=588288&view=diff
==============================================================================
--- webservices/axis2/trunk/java/modules/jaxws/src/org/apache/axis2/jaxws/message/databinding/JAXBUtils.java (original)
+++ webservices/axis2/trunk/java/modules/jaxws/src/org/apache/axis2/jaxws/message/databinding/JAXBUtils.java Thu Oct 25 10:37:27 2007
@@ -51,14 +51,13 @@
 
 
 /**
- * JAXB Utilites to pool JAXBContext and related objects. Currently the JAXBContext is pooled by
- * Class name.  We may change this to create and pool by package name.
+ * JAXB Utilites to pool JAXBContext and related objects. 
  */
 public class JAXBUtils {
 
     private static final Log log = LogFactory.getLog(JAXBUtils.class);
 
-    // Create a concurrent map to get the JAXBObject: keys are ClassLoader and Set<String>.
+    // Create a concurrent map to get the JAXBObject: keys are ClassLoader and String (package names).
     private static Map<ClassLoader, Map<String, JAXBContextValue>> jaxbMap =
             new ConcurrentHashMap<ClassLoader, Map<String, JAXBContextValue>>();
 
@@ -78,7 +77,8 @@
     // In that case, consider pooling Unmarshaller objects.
     // Different threads may reuse one Unmarshaller instance, 
     // as long as you don't use one instance from two threads at the same time. 
-
+    // ENABLE_ADV_POOLING is false...which means they are obtained from the JAXBContext instead of
+    // from the pool.
     private static boolean ENABLE_ADV_POOLING = false;
 
     // The maps are freed up when a LOAD FACTOR is hit
@@ -105,6 +105,9 @@
 
     /**
      * Get a JAXBContext for the class
+     * 
+     * Note: The contextPackage object is used by multiple threads.  It should be considered immutable
+     * and not altered by this method.
      *
      * @param contextPackage Set<Package>
      * @param cacheKey ClassLoader
@@ -128,7 +131,10 @@
     /**
      * Get a JAXBContext for the class
      *
-     * @param contextPackage  Set<Package>
+     * Note: The contextPackage object is used by multiple threads.  It should be considered immutable
+     * and not altered by this method.
+     * 
+     * @param contextPackage  Set<Package> 
      * @param contructionType (output value that indicates how the context was constructed)
      * @param cacheKey ClassLoader
      * @return JAXBContext
@@ -179,21 +185,26 @@
         if (contextValue == null) {
             adjustPoolSize(innerMap);
 
-            // A pooled context was not found, so create one and put it in the map.
-            synchronized(contextPackages) {
-               // synchronized on contextPackages because this method may prune the contextPackages
-               contextValue = createJAXBContextValue(contextPackages, cl);
-            }
-            // Put the new context in the map keyed by both the original and current list of packages
+            // Create a copy of the contextPackages.  This new TreeSet will
+            // contain only the valid contextPackages.
+            // Note: The original contextPackage set is accessed by multiple 
+            // threads and should not be altered.
+            
+            TreeSet<String> validContextPackages = new TreeSet<String>(contextPackages);  
+            contextValue = createJAXBContextValue(validContextPackages, cl);
+            
+            // Put the new context in the map keyed by both the original and valid list of packages
+            String validPackagesKey = validContextPackages.toString();
             innerMap.put(key, contextValue);
-            innerMap.put(contextPackages.toString(), contextValue);
+            innerMap.put(validPackagesKey, contextValue);
             if (log.isDebugEnabled()) {
-                log.debug("JAXBContext [created] for " + contextPackages.toString());
+                log.debug("JAXBContext [created] for " + key);
+                log.debug("JAXBContext also stored by the list of valid packages:" + validPackagesKey);
             }
 
         } else {
             if (log.isDebugEnabled()) {
-                log.debug("JAXBContext [from pool] for " + contextPackages.toString());
+                log.debug("JAXBContext [from pool] for " + key);
             }
         }
         constructionType.value = contextValue.constructionType;

Modified: webservices/axis2/trunk/java/modules/jaxws/test/org/apache/axis2/jaxws/misc/JAXBContextTest.java
URL: http://svn.apache.org/viewvc/webservices/axis2/trunk/java/modules/jaxws/test/org/apache/axis2/jaxws/misc/JAXBContextTest.java?rev=588288&r1=588287&r2=588288&view=diff
==============================================================================
--- webservices/axis2/trunk/java/modules/jaxws/test/org/apache/axis2/jaxws/misc/JAXBContextTest.java (original)
+++ webservices/axis2/trunk/java/modules/jaxws/test/org/apache/axis2/jaxws/misc/JAXBContextTest.java Thu Oct 25 10:37:27 2007
@@ -85,8 +85,9 @@
         assertTrue(jaxbContext1.toString().equals(jaxbContext1.toString()));
         assertTrue(context3.contains("org.test.addnumbers"));
         assertTrue(context3.contains("org.test.anytype")); 
-        // TODO FIXME - does not work under m2/surefire
-//        assertTrue(!context3.contains("my.grandma.loves.jaxws"));  // invalid package should be silently removed
+        // The invalid package should now be retained...this is due
+        // a minor semantic change to avoid this side effect.
+        assertTrue(context3.contains("my.grandma.loves.jaxws"));  
         
         // Repeat with a subset of packages
         TreeSet<String> context4 = new TreeSet<String>();



---------------------------------------------------------------------
To unsubscribe, e-mail: axis-cvs-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-cvs-help@ws.apache.org