tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1527472 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/tomcat/util/net/AprEndpoint.java webapps/docs/changelog.xml
Date Mon, 30 Sep 2013 07:45:09 GMT
Author: markt
Date: Mon Sep 30 07:45:09 2013
New Revision: 1527472

URL: http://svn.apache.org/r1527472
Log:
Removal of a socket from the poller needs to happen on the poller thread as removal, like
addition, is not thread safe.
Removal of a socket from the poller needs to remove the socket from the addListif it is present
otherwise the closed socket could end up in the poller.

Modified:
    tomcat/tc7.0.x/trunk/   (props changed)
    tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
    tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml

Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
  Merged /tomcat/trunk:r1526052

Modified: tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=1527472&r1=1527471&r2=1527472&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Mon Sep 30 07:45:09
2013
@@ -899,7 +899,7 @@ public class AprEndpoint extends Abstrac
         // countDownConnection() in that case
         Poller poller = this.poller;
         if (poller != null) {
-            poller.removeFromPoller(socket);
+            poller.remove(socket);
         }
         connections.remove(Long.valueOf(socket));
         destroySocket(socket, running);
@@ -1212,6 +1212,19 @@ public class AprEndpoint extends Abstrac
             }
         }
 
+        public boolean remove(long socket) {
+            for (int i = 0; i < size; i++) {
+                if (sockets[i] == socket) {
+                    sockets[i] = sockets[size - 1];
+                    timeouts[i] = timeouts[size - 1];
+                    flags[size] = flags[size -1];
+                    size--;
+                    return true;
+                }
+            }
+            return false;
+        }
+
         public void duplicate(SocketList copy) {
             copy.size = size;
             copy.pos = pos;
@@ -1268,6 +1281,12 @@ public class AprEndpoint extends Abstrac
 
         
         /**
+         * List of sockets to be removed from the poller.
+         */
+        private SocketList removeList = null;
+
+
+        /**
          * Structure used for storing timeouts.
          */
         protected SocketTimeouts timeouts = null;
@@ -1341,6 +1360,7 @@ public class AprEndpoint extends Abstrac
             desc = new long[actualPollerSize * 2];
             connectionCount = 0;
             addList = new SocketList(defaultPollerSize);
+            removeList = new SocketList(defaultPollerSize);
         }
 
 
@@ -1456,8 +1476,10 @@ public class AprEndpoint extends Abstrac
             }
         }
 
+
         /**
-         * Add specified socket to one of the pollers.
+         * Add specified socket to one of the pollers. Must only be called from
+         * {@link Poller#run()}.
          */
         protected boolean addToPoller(long socket, int events) {
             int rv = -1;
@@ -1474,10 +1496,19 @@ public class AprEndpoint extends Abstrac
             return false;
         }
 
+
+        protected void remove(long socket) {
+            synchronized (this) {
+                removeList.add(socket, 0, 0);
+            }
+        }
+
+
         /**
-         * Remove specified socket from the pollers.
+         * Remove specified socket from the pollers. Must only be called from
+         * {@link Poller#run()}.
          */
-        protected boolean removeFromPoller(long socket) {
+        private boolean removeFromPoller(long socket) {
             int rv = -1;
             for (int i = 0; i < pollers.length; i++) {
                 if (pollerSpace[i] < actualPollerSize) {
@@ -1552,7 +1583,7 @@ public class AprEndpoint extends Abstrac
 
             int maintain = 0;
             SocketList localAddList = new SocketList(getMaxConnections());
-
+            SocketList localRemoveList = new SocketList(getMaxConnections());
 
             // Loop until we receive a shutdown command
             while (pollerRunning) {
@@ -1590,7 +1621,18 @@ public class AprEndpoint extends Abstrac
                 }
 
                 try {
-                    // Add sockets which are waiting to the poller
+                    // Duplicate the add and remove lists so that the syncs are
+                    // minimised
+                    if (removeList.size() > 0) {
+                        synchronized (this) {
+                            // Duplicate to another list, so that the syncing is
+                            // minimal
+                            removeList.duplicate(localRemoveList);
+                            removeList.clear();
+                        }
+                    } else {
+                        localAddList.clear();
+                    }
                     if (addList.size() > 0) {
                         synchronized (this) {
                             // Duplicate to another list, so that the syncing is
@@ -1598,6 +1640,22 @@ public class AprEndpoint extends Abstrac
                             addList.duplicate(localAddList);
                             addList.clear();
                         }
+                    } else {
+                        localAddList.clear();
+                    }
+
+                    // Remove sockets
+                    if (localRemoveList.size() > 0) {
+                        SocketInfo info = localRemoveList.get();
+                        while (info != null) {
+                            localAddList.remove(info.socket);
+                            removeFromPoller(info.socket);
+                            info = localRemoveList.get();
+                        }
+                    }
+
+                    // Add sockets which are waiting to the poller
+                    if (localAddList.size() > 0) {
                         SocketInfo info = localAddList.get();
                         while (info != null) {
                             if (log.isDebugEnabled()) {

Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1527472&r1=1527471&r2=1527472&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Mon Sep 30 07:45:09 2013
@@ -59,9 +59,13 @@
   <subsection name="Coyote">
     <changelog>
       <scode>
-        Refactor APR endpoint to reduce scope of <code>localAddList</code>.
-        (markt)
+        Refactor APR/native connector to reduce the scope of
+        <code>localAddList</code>. (markt)
       </scode>
+      <fix>
+        Ensure that sockets removed from the Poller in the APR/native connector
+        are removed in a thread-safe manner. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Cluster">



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


Mime
View raw message