On Sat, May 20, 2023 at 7:19 PM Mark Thomas <ma...@apache.org> wrote: > > 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.
Seconded since the test failures caught this. > > --- > > 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. I never did it on purpose since it seemed useless. However, I don't think it really makes any difference. Rémy > > 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 > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org