Return-Path: <dev-return-187498-archive-asf-public=cust-asf.ponee.io@tomcat.apache.org>
X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io
Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io
Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183])
	by cust-asf2.ponee.io (Postfix) with ESMTP id 4E3F6200D50
	for <archive-asf-public-internal@cust-asf2.ponee.io>; Mon,  4 Dec 2017 13:47:01 +0100 (CET)
Received: by cust-asf.ponee.io (Postfix)
	id 4CA71160C05; Mon,  4 Dec 2017 12:47:01 +0000 (UTC)
Delivered-To: archive-asf-public@cust-asf.ponee.io
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by cust-asf.ponee.io (Postfix) with SMTP id 929DA160BF9
	for <archive-asf-public@cust-asf.ponee.io>; Mon,  4 Dec 2017 13:47:00 +0100 (CET)
Received: (qmail 25778 invoked by uid 500); 4 Dec 2017 12:46:59 -0000
Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:dev-help@tomcat.apache.org>
List-Unsubscribe: <mailto:dev-unsubscribe@tomcat.apache.org>
List-Post: <mailto:dev@tomcat.apache.org>
List-Id: <dev.tomcat.apache.org>
Reply-To: "Tomcat Developers List" <dev@tomcat.apache.org>
Delivered-To: mailing list dev@tomcat.apache.org
Received: (qmail 25767 invoked by uid 99); 4 Dec 2017 12:46:59 -0000
Received: from Unknown (HELO svn01-us-west.apache.org) (209.188.14.144)
    by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 04 Dec 2017 12:46:59 +0000
Received: from svn01-us-west.apache.org (localhost [127.0.0.1])
	by svn01-us-west.apache.org (ASF Mail Server at svn01-us-west.apache.org) with ESMTP id D6CC43A00E7
	for <dev@tomcat.apache.org>; Mon,  4 Dec 2017 12:46:57 +0000 (UTC)
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: 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
Date: Mon, 04 Dec 2017 12:46:55 -0000
To: dev@tomcat.apache.org
From: markt@apache.org
X-Mailer: svnmailer-1.0.9
Message-Id: <20171204124657.D6CC43A00E7@svn01-us-west.apache.org>
archived-at: Mon, 04 Dec 2017 12:47:01 -0000

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