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: [email protected]
For additional commands, e-mail: [email protected]
