You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2017/12/04 12:46:55 UTC

svn commit: r1817088 - in /tomcat/trunk: java/org/apache/catalina/tribes/membership/Membership.java res/findbugs/filter-false-positives.xml test/org/apache/catalina/tribes/membership/TestMembership.java

Author: markt
Date: Mon Dec  4 12:46:55 2017
New Revision: 1817088

URL: http://svn.apache.org/viewvc?rev=1817088&view=rev
Log:
Refactor so clone() works correctly for sub-classes (it seemed reasonable that this might get sub-classed)

Modified:
    tomcat/trunk/java/org/apache/catalina/tribes/membership/Membership.java
    tomcat/trunk/res/findbugs/filter-false-positives.xml
    tomcat/trunk/test/org/apache/catalina/tribes/membership/TestMembership.java

Modified: tomcat/trunk/java/org/apache/catalina/tribes/membership/Membership.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/tribes/membership/Membership.java?rev=1817088&r1=1817087&r2=1817088&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/tribes/membership/Membership.java (original)
+++ tomcat/trunk/java/org/apache/catalina/tribes/membership/Membership.java Mon Dec  4 12:46:55 2017
@@ -36,7 +36,8 @@ public class Membership implements Clone
 
     protected static final Member[] EMPTY_MEMBERS = new Member[0];
 
-    private final Object membersLock = new Object();
+    // Non-final to support clone()
+    private Object membersLock = new Object();
 
     /**
      * The local member.
@@ -59,13 +60,29 @@ public class Membership implements Clone
     protected final Comparator<Member> memberComparator;
 
     @Override
-    public Object clone() {
+    public Membership clone() {
         synchronized (membersLock) {
-            Membership clone = new Membership(local, memberComparator);
+            Membership clone;
+            try {
+                clone = (Membership) super.clone();
+            } catch (CloneNotSupportedException e) {
+                // Can't happen
+                throw new AssertionError();
+            }
+
+            // Standard clone() method will copy the map object. Replace that
+            // with a new map but with the same contents.
             @SuppressWarnings("unchecked")
             final HashMap<Member, MbrEntry> tmpclone = (HashMap<Member, MbrEntry>) map.clone();
             clone.map = tmpclone;
+
+            // Standard clone() method will copy the array obejct. Replace that
+            // with a new array but with the same contents.
             clone.members = members.clone();
+
+            // Standard clone() method will copy the lock object. Replace that
+            // with a new object.
+            clone.membersLock = new Object();
             return clone;
         }
     }

Modified: tomcat/trunk/res/findbugs/filter-false-positives.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/res/findbugs/filter-false-positives.xml?rev=1817088&r1=1817087&r2=1817088&view=diff
==============================================================================
--- tomcat/trunk/res/findbugs/filter-false-positives.xml (original)
+++ tomcat/trunk/res/findbugs/filter-false-positives.xml Mon Dec  4 12:46:55 2017
@@ -438,6 +438,12 @@
     <Bug pattern="UG_SYNC_SET_UNSYNC_GET"/>
   </Match>
   <Match>
+    <!-- lock is in clone so this is safe -->
+    <Class name="org.apache.catalina.tribes.membership.Membership" />
+    <Method name="clone" />
+    <Bug pattern="ML_SYNC_ON_FIELD_TO_GUARD_CHANGING_THAT_FIELD" />
+  </Match>
+  <Match>
     <!-- Intentional in case thread is waiting -->
     <Class name="org.apache.catalina.tribes.transport.RxTaskPool"/>
     <Method name="returnWorker"/>

Modified: tomcat/trunk/test/org/apache/catalina/tribes/membership/TestMembership.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/tribes/membership/TestMembership.java?rev=1817088&r1=1817087&r2=1817088&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/tribes/membership/TestMembership.java (original)
+++ tomcat/trunk/test/org/apache/catalina/tribes/membership/TestMembership.java Mon Dec  4 12:46:55 2017
@@ -38,7 +38,7 @@ public class TestMembership {
         original.addMember(m2);
         original.addMember(m3);
 
-        Membership clone = (Membership) original.clone();
+        Membership clone = original.clone();
 
         Assert.assertFalse(original == clone);
         Assert.assertTrue(original.getClass() == clone.getClass());



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org