On 19/05/2023 04:24, li...@apache.org wrote:
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/main by this push:
      new 10492dd22b Clear SocketWrapper reference to help GC
10492dd22b is described below

commit 10492dd22bd64ff63cf77786fa67d45cdc2a54b3
Author: lihan <li...@apache.org>
AuthorDate: Fri May 19 11:24:27 2023 +0800

     Clear SocketWrapper reference to help GC

-1. Veto.

---
  java/org/apache/coyote/AbstractProcessor.java      | 3 +++
  java/org/apache/coyote/http11/Http11Processor.java | 1 -
  java/org/apache/coyote/http2/StreamProcessor.java  | 6 +-----

The above changes need to be reverted since they break AJP connections. The AJP processor is recycled after CPING MESSAGES and expects processing to continue afterwards. Setting the SocketWrapper to null triggers NPEs when trying to process the request that follows the CPING.

  java/org/apache/tomcat/util/net/Nio2Channel.java   | 1 +
  java/org/apache/tomcat/util/net/NioChannel.java    | 1 +

The above changes look to be OK although I'll note that they do add some processing overhead.

Mark


  5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/java/org/apache/coyote/AbstractProcessor.java 
b/java/org/apache/coyote/AbstractProcessor.java
index 3ee6898fbd..8965dab62b 100644
--- a/java/org/apache/coyote/AbstractProcessor.java
+++ b/java/org/apache/coyote/AbstractProcessor.java
@@ -721,6 +721,9 @@ public abstract class AbstractProcessor extends 
AbstractProcessorLight implement
      public void recycle() {
          errorState = ErrorState.NONE;
          asyncStateMachine.recycle();
+        // Clear fields that can be cleared to aid GC and trigger NPEs if this
+        // is reused
+        socketWrapper = null;
      }
diff --git a/java/org/apache/coyote/http11/Http11Processor.java b/java/org/apache/coyote/http11/Http11Processor.java
index 9e2e74914c..85dfd34f3c 100644
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -1418,7 +1418,6 @@ public class Http11Processor extends AbstractProcessor {
          inputBuffer.recycle();
          outputBuffer.recycle();
          upgradeToken = null;
-        socketWrapper = null;
          sendfileData = null;
          sslSupport = null;
      }
diff --git a/java/org/apache/coyote/http2/StreamProcessor.java 
b/java/org/apache/coyote/http2/StreamProcessor.java
index 78cb770649..3b1c03ac38 100644
--- a/java/org/apache/coyote/http2/StreamProcessor.java
+++ b/java/org/apache/coyote/http2/StreamProcessor.java
@@ -392,8 +392,8 @@ class StreamProcessor extends AbstractProcessor {
@Override
      public final void recycle() {
+        super.recycle();
          // StreamProcessor instances are not re-used.
-
          // Calling removeRequestProcessor even though the RequestProcesser was
          // never added will add the values from the RequestProcessor to the
          // running total for the GlobalRequestProcessor
@@ -401,10 +401,6 @@ class StreamProcessor extends AbstractProcessor {
          if (global != null) {
              global.removeRequestProcessor(request.getRequestProcessor());
          }
-
-        // Clear fields that can be cleared to aid GC and trigger NPEs if this
-        // is reused
-        setSocketWrapper(null);
      }
diff --git a/java/org/apache/tomcat/util/net/Nio2Channel.java b/java/org/apache/tomcat/util/net/Nio2Channel.java
index 603c43f416..9d65266374 100644
--- a/java/org/apache/tomcat/util/net/Nio2Channel.java
+++ b/java/org/apache/tomcat/util/net/Nio2Channel.java
@@ -79,6 +79,7 @@ public class Nio2Channel implements AsynchronousByteChannel {
      @Override
      public void close() throws IOException {
          sc.close();
+        reset(this.sc, null);
      }
diff --git a/java/org/apache/tomcat/util/net/NioChannel.java b/java/org/apache/tomcat/util/net/NioChannel.java
index d263ce9ae6..eae3f51171 100644
--- a/java/org/apache/tomcat/util/net/NioChannel.java
+++ b/java/org/apache/tomcat/util/net/NioChannel.java
@@ -81,6 +81,7 @@ public class NioChannel implements ByteChannel, 
ScatteringByteChannel, Gathering
      @Override
      public void close() throws IOException {
          sc.close();
+        reset(this.sc,null);
      }
/**


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


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

Reply via email to