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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org