tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r...@apache.org
Subject [tomcat] branch master updated: Remove connections map from APR endpoint
Date Wed, 06 Nov 2019 10:01:58 GMT
This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
     new 4945083  Remove connections map from APR endpoint
4945083 is described below

commit 49450836423be8ed1d1e3b6a46757f983ede9414
Author: remm <remm@apache.org>
AuthorDate: Wed Nov 6 11:01:43 2019 +0100

    Remove connections map from APR endpoint
    
    The connections map from AbstractEndpoint can be used by APR as well,
    after refactoring using a key on the socket. Once (if) APR is removed,
    this can go back to using the socket wrapper as a key (and value).
    AbstractEndpoint.getConnections becomes more costly but it is only used
    on pause or destroy.
    Also make sure close attempts a full cleanup using a finally to remove
    from the connection map and call doClose.
---
 .../org/apache/tomcat/util/net/AbstractEndpoint.java | 11 ++++++++---
 java/org/apache/tomcat/util/net/AprEndpoint.java     | 20 ++++++++------------
 java/org/apache/tomcat/util/net/Nio2Endpoint.java    |  2 +-
 java/org/apache/tomcat/util/net/NioEndpoint.java     |  2 +-
 .../apache/tomcat/util/net/SocketWrapperBase.java    |  5 +++--
 webapps/docs/changelog.xml                           |  9 +++++----
 6 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/AbstractEndpoint.java b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
index f3f10a4..c900006 100644
--- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
@@ -25,6 +25,7 @@ import java.net.SocketException;
 import java.util.ArrayList;
 import java.util.Enumeration;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -188,12 +189,16 @@ public abstract class AbstractEndpoint<S,U> {
     private ObjectName oname = null;
 
     /**
-     * Connection structure holding all current connections.
+     * Map holding all current connections keyed with the sockets (for APR).
      */
-    protected Map<SocketWrapperBase<S>, SocketWrapperBase<S>> connections
= new ConcurrentHashMap<>();
+    protected Map<S, SocketWrapperBase<S>> connections = new ConcurrentHashMap<>();
 
+    /**
+     * Get a set with the current open connections.
+     * @return A set with the open socket wrappers
+     */
     public Set<SocketWrapperBase<S>> getConnections() {
-        return connections.keySet();
+        return new HashSet<SocketWrapperBase<S>>(connections.values());
     }
 
     // ----------------------------------------------------------------- Properties
diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java
index 5629154..1fefe85 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -27,7 +27,6 @@ import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
@@ -109,9 +108,6 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements
SNICallB
     protected long sslContext = 0;
 
 
-    private final Map<Long,AprSocketWrapper> connections = new ConcurrentHashMap<>();
-
-
     // ------------------------------------------------------------ Constructor
 
     public AprEndpoint() {
@@ -684,7 +680,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements
SNICallB
                 log.debug(sm.getString("endpoint.debug.socket", socket));
             }
             AprSocketWrapper wrapper = new AprSocketWrapper(socket, this);
-            super.connections.put(wrapper, wrapper);
+            connections.put(socket, wrapper);
             wrapper.setKeepAliveLeft(getMaxKeepAliveRequests());
             wrapper.setSecure(isSSLEnabled());
             wrapper.setReadTimeout(getConnectionTimeout());
@@ -729,7 +725,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements
SNICallB
      *         socket should be closed
      */
     protected boolean processSocket(long socket, SocketEvent event) {
-        AprSocketWrapper socketWrapper = connections.get(Long.valueOf(socket));
+        SocketWrapperBase<Long> socketWrapper = connections.get(Long.valueOf(socket));
         if (event == SocketEvent.OPEN_READ && socketWrapper.readOperation != null)
{
             return socketWrapper.readOperation.process();
         } else if (event == SocketEvent.OPEN_WRITE && socketWrapper.writeOperation
!= null) {
@@ -780,7 +776,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements
SNICallB
      * this directly from a known error condition.
      */
     private void destroySocket(long socket) {
-        connections.remove(Long.valueOf(socket));
+        closeSocket(socket);
         if (log.isDebugEnabled()) {
             String msg = sm.getString("endpoint.debug.destroySocket",
                     Long.valueOf(socket));
@@ -1252,7 +1248,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements
SNICallB
                     log.debug(sm.getString("endpoint.debug.socketTimeout",
                             Long.valueOf(socket)));
                 }
-                AprSocketWrapper socketWrapper = connections.get(Long.valueOf(socket));
+                SocketWrapperBase<Long> socketWrapper = connections.get(Long.valueOf(socket));
                 if (socketWrapper != null) {
                     socketWrapper.setError(new SocketTimeoutException());
                     if (socketWrapper.readOperation != null || socketWrapper.writeOperation
!= null) {
@@ -1377,8 +1373,8 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements
SNICallB
                                         Long.valueOf(info.socket)));
                             }
                             timeouts.remove(info.socket);
-                            AprSocketWrapper wrapper = connections.get(
-                                    Long.valueOf(info.socket));
+                            AprSocketWrapper wrapper =
+                                    (AprSocketWrapper) connections.get(Long.valueOf(info.socket));
                             if (wrapper != null) {
                                 if (info.read() || info.write()) {
                                     wrapper.pollerFlags = wrapper.pollerFlags |
@@ -1423,8 +1419,8 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements
SNICallB
                                         Long.valueOf(desc[n*2])));
                             }
                             long timeout = timeouts.remove(desc[n*2+1]);
-                            AprSocketWrapper wrapper = connections.get(
-                                    Long.valueOf(desc[n*2+1]));
+                            AprSocketWrapper wrapper = (AprSocketWrapper)
+                                    connections.get(Long.valueOf(desc[n*2+1]));
                             if (wrapper == null) {
                                 // Socket was closed in another thread while still in
                                 // the Poller but wasn't removed from the Poller before
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 1ac7025..afb8757 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -320,7 +320,7 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS
                 }
             }
             Nio2SocketWrapper socketWrapper = new Nio2SocketWrapper(channel, this);
-            connections.put(socketWrapper, socketWrapper);
+            connections.put(channel, socketWrapper);
             channel.reset(socket, socketWrapper);
             socketWrapper.setReadTimeout(getConnectionTimeout());
             socketWrapper.setWriteTimeout(getConnectionTimeout());
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java
index 9ba8262..0cb33f6 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -417,7 +417,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
             } else {
             }
             NioSocketWrapper socketWrapper = new NioSocketWrapper(channel, this);
-            connections.put(socketWrapper, socketWrapper);
+            connections.put(channel, socketWrapper);
             channel.reset(socket, socketWrapper);
             socketWrapper.setReadTimeout(getConnectionTimeout());
             socketWrapper.setWriteTimeout(getConnectionTimeout());
diff --git a/java/org/apache/tomcat/util/net/SocketWrapperBase.java b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
index cb9460e..2f4fb42 100644
--- a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
+++ b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
@@ -408,9 +408,10 @@ public abstract class SocketWrapperBase<E> {
                 if (log.isDebugEnabled()) {
                     log.error(sm.getString("endpoint.debug.handlerRelease"), e);
                 }
+            } finally {
+                getEndpoint().connections.remove(socket);
+                doClose();
             }
-            getEndpoint().connections.remove(this);
-            doClose();
         }
     }
 
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 421b323..851b817 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -105,10 +105,11 @@
         <bug>63879</bug>: Remove stack trace from debug logging on socket
         wrapper close. (remm)
       </fix>
-      <fix>
-        Move connection tracking to the endpoint, since it requires far fewer
-        operations. (remm)
-      </fix>
+      <update>
+        Add connection tracking on the connector endpoint to remove excessive
+        concurrency in the protocol handler when maintaining an association
+        between the socket wrapper and its current processor. (remm)
+      </update>
       <fix>
         <bug>63894</bug>: Ensure that the configured values for
         <code>certificateVerification</code> and


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


Mime
View raw message