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 2013/10/06 00:21:40 UTC

svn commit: r1529537 - in /tomcat/trunk/java/org/apache: coyote/AbstractProtocol.java tomcat/util/net/AbstractEndpoint.java tomcat/util/net/SocketWrapper.java

Author: markt
Date: Sat Oct  5 22:21:40 2013
New Revision: 1529537

URL: http://svn.apache.org/r1529537
Log:
Make access of dispatcher list on socket wrapper thread safe

Modified:
    tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java
    tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java
    tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapper.java

Modified: tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java?rev=1529537&r1=1529536&r2=1529537&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java (original)
+++ tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java Sat Oct  5 22:21:40 2013
@@ -18,6 +18,7 @@ package org.apache.coyote;
 
 import java.io.IOException;
 import java.net.InetAddress;
+import java.util.Iterator;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.Executor;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -616,14 +617,19 @@ public abstract class AbstractProtocol<S
                 initSsl(wrapper, processor);
 
                 SocketState state = SocketState.CLOSED;
+                Iterator<DispatchType> dispatches = null;
                 do {
-                    if (wrapper.hasNextDispatch()) {
-                        // Associate with the processor with the connection as
-                        // these calls may result in a nested call to process()
-                        connections.put(socket, processor);
-                        DispatchType nextDispatch = wrapper.getNextDispatch();
-                        state = processor.asyncDispatch(
-                                nextDispatch.getSocketStatus());
+                    if (dispatches != null) {
+                        if (dispatches.hasNext()) {
+                            // Associate with the processor with the connection as
+                            // these calls may result in a nested call to process()
+                            connections.put(socket, processor);
+                            DispatchType nextDispatch = dispatches.next();
+                            state = processor.asyncDispatch(
+                                    nextDispatch.getSocketStatus());
+                        } else {
+                            dispatches = null;
+                        }
                     } else if (status == SocketStatus.DISCONNECT &&
                             !processor.isComet()) {
                         // Do nothing here, just wait for it to get recycled
@@ -670,9 +676,12 @@ public abstract class AbstractProtocol<S
                                 "], Status in: [" + status +
                                 "], State out: [" + state + "]");
                     }
+                    if (dispatches == null || !dispatches.hasNext()) {
+                        dispatches = wrapper.getIteratorAndClearDispatches();
+                    }
                 } while (state == SocketState.ASYNC_END ||
                         state == SocketState.UPGRADING ||
-                        wrapper.hasNextDispatch() && state != SocketState.CLOSED);
+                        dispatches.hasNext() && state != SocketState.CLOSED);
 
                 if (state == SocketState.LONG) {
                     // In the middle of processing a request/response. Keep the

Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java?rev=1529537&r1=1529536&r2=1529537&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java Sat Oct  5 22:21:40 2013
@@ -23,6 +23,7 @@ import java.net.InetSocketAddress;
 import java.security.PrivilegedAction;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.StringTokenizer;
 import java.util.concurrent.Executor;
 import java.util.concurrent.TimeUnit;
@@ -636,13 +637,11 @@ public abstract class AbstractEndpoint<S
 
 
     public void executeNonBlockingDispatches(SocketWrapper<S> socketWrapper) {
-        // Synchronise on the socket wrapper to ensure no other threads are
-        // working with the socket
-        synchronized (socketWrapper) {
-            while (socketWrapper.hasNextDispatch()) {
-                DispatchType dispatchType = socketWrapper.getNextDispatch();
-                processSocket(socketWrapper, dispatchType.getSocketStatus(), false);
-            }
+        Iterator<DispatchType> dispatches = socketWrapper.getIteratorAndClearDispatches();
+
+        while (dispatches.hasNext()) {
+            DispatchType dispatchType = dispatches.next();
+            processSocket(socketWrapper, dispatchType.getSocketStatus(), false);
         }
     }
 

Modified: tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapper.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapper.java?rev=1529537&r1=1529536&r2=1529537&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapper.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapper.java Sat Oct  5 22:21:40 2013
@@ -17,8 +17,8 @@
 package org.apache.tomcat.util.net;
 
 import java.util.Iterator;
-import java.util.LinkedHashSet;
 import java.util.Set;
+import java.util.concurrent.CopyOnWriteArraySet;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock;
@@ -62,7 +62,7 @@ public class SocketWrapper<E> {
      */
     private final Object writeThreadLock = new Object();
 
-    private Set<DispatchType> dispatches = new LinkedHashSet<>();
+    private Set<DispatchType> dispatches = new CopyOnWriteArraySet<>();
 
     public SocketWrapper(E socket) {
         this.socket = socket;
@@ -114,22 +114,22 @@ public class SocketWrapper<E> {
     }
     public Object getWriteThreadLock() { return writeThreadLock; }
     public void addDispatch(DispatchType dispatchType) {
-        dispatches.add(dispatchType);
-    }
-    public boolean hasNextDispatch() {
-        return dispatches.size() > 0;
+        synchronized (dispatches) {
+            dispatches.add(dispatchType);
+        }
     }
-    public DispatchType getNextDispatch() {
-        DispatchType result = null;
-        Iterator<DispatchType> iter = dispatches.iterator();
-        if (iter.hasNext()) {
-            result = iter.next();
-            iter.remove();
+    public Iterator<DispatchType> getIteratorAndClearDispatches() {
+        Iterator<DispatchType> result;
+        synchronized (dispatches) {
+            result = dispatches.iterator();
+            dispatches.clear();
         }
         return result;
     }
     public void clearDispatches() {
-        dispatches.clear();
+        synchronized (dispatches) {
+            dispatches.clear();
+        }
     }
 
     public void reset(E socket, long timeout) {



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