You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@abdera.apache.org by ro...@apache.org on 2006/08/09 00:09:46 UTC

svn commit: r429850 - /incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax/FOMParser.java

Author: rooneg
Date: Tue Aug  8 15:09:45 2006
New Revision: 429850

URL: http://svn.apache.org/viewvc?rev=429850&view=rev
Log:
Tweak the behavior of get/set DefaultParserOptions for sanity and thread
safety.

* parser/src/main/java/org/apache/abdera/parser/stax/FOMParser.java
  (getDefaultParserOptions): Note why we're cloning the options, and
   if we get a CloneNotSupportedException just throw a RuntimeException,
   since that's really a bogus ParserOptions impl, and we shouldn't let
   it silently continue to suck.
  (setDefaultParserOptions): Make a defensive copy of the options, so
   that we remain thread safe even if the calling thread later messes
   with it.  Comment about why, and add the same exception handling we
   use in the previous method for the same reasons.

Modified:
    incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax/FOMParser.java

Modified: incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax/FOMParser.java
URL: http://svn.apache.org/viewvc/incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax/FOMParser.java?rev=429850&r1=429849&r2=429850&view=diff
==============================================================================
--- incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax/FOMParser.java (original)
+++ incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax/FOMParser.java Tue Aug  8 15:09:45 2006
@@ -131,19 +131,31 @@
   public synchronized ParserOptions getDefaultParserOptions() {
     if (options == null)
       options = new FOMParserOptions();
-    
+
+    // Make a copy of the options, so that changes to it don't result in
+    // changes to the Parser's defaults.  Also, this allows us to remain
+    // thread safe without having to make ParseOptions implementations
+    // synchronized.
+
     try {
       return (ParserOptions) options.clone();
     } catch (CloneNotSupportedException cnse) {
-      // Nothing... 
+      // This shouldn't actually happen
+      throw new RuntimeException(cnse);
     }
-    
-    // About the best we can do if clone doesn't work :(
-    return new FOMParserOptions();
   }
 
   public synchronized void setDefaultParserOptions(ParserOptions options) {
-    this.options = options;
+    // Ok, we need to make a defensive copy of the options, since otherwise
+    // the caller still has access to the object, which means our access to
+    // it isn't certain to be thread safe.
+
+    try {
+      this.options = (ParserOptions) options.clone();
+    } catch (CloneNotSupportedException cnse) {
+      // This shouldn't actually happen
+      throw new RuntimeException(cnse);
+    }
   }
 
 }