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 c2d6278  Refactor socket wrapper close
c2d6278 is described below

commit c2d6278b339384f9e6679b718ceb861d0329be1d
Author: remm <r...@apache.org>
AuthorDate: Wed May 15 13:40:49 2019 +0200

    Refactor socket wrapper close
    
    Redo again close processing using an atomic boolean and a doClose method
    that subclasses will implement, with a guarantee that it will be run
    only once. Improve slightly NIO close with respect to recycling the
    NioChannel. APR will use atomic boolean object for locking instead of
    closedLock.
    Once the NIOx socket wrapper is closed, the channel will now be replaced
    by a dummy closed channel. Also all the buffers linked will be replaced
    with empty ones.
---
 java/org/apache/tomcat/util/net/AprEndpoint.java   | 72 ++++++++----------
 java/org/apache/tomcat/util/net/Nio2Channel.java   | 85 +++++++++++++++++++++-
 java/org/apache/tomcat/util/net/Nio2Endpoint.java  | 37 ++++------
 java/org/apache/tomcat/util/net/NioChannel.java    | 52 +++++++++++--
 java/org/apache/tomcat/util/net/NioEndpoint.java   | 74 ++++++++-----------
 .../tomcat/util/net/SocketBufferHandler.java       |  6 ++
 .../apache/tomcat/util/net/SocketWrapperBase.java  | 32 +++++++-
 java/org/apache/tomcat/util/net/WriteBuffer.java   |  3 +
 webapps/docs/changelog.xml                         | 12 +++
 9 files changed, 254 insertions(+), 119 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java 
b/java/org/apache/tomcat/util/net/AprEndpoint.java
index df85e6c..9551b7e 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -494,11 +494,7 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
             running = false;
             poller.stop();
             for (SocketWrapperBase<Long> socketWrapper : connections.values()) 
{
-                try {
-                    socketWrapper.close();
-                } catch (IOException e) {
-                    // Ignore
-                }
+                socketWrapper.close();
             }
             long waitLeft = 10000;
             while (waitLeft > 0 &&
@@ -2150,9 +2146,6 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
 
         private final ByteBuffer sslOutputBuffer;
 
-        private final Object closedLock = new Object();
-        private volatile boolean closed = false;
-
         // This field should only be used by Poller#run()
         private int pollerFlags = 0;
 
@@ -2246,7 +2239,7 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
 
 
         private int fillReadBuffer(boolean block, ByteBuffer to) throws 
IOException {
-            if (closed) {
+            if (isClosed()) {
                 throw new IOException(sm.getString("socket.apr.closed", 
getSocket()));
             }
 
@@ -2343,15 +2336,18 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
 
 
         @Override
-        public void close() {
-            getEndpoint().getHandler().release(this);
-            synchronized (closedLock) {
-                // APR typically crashes if the same socket is closed twice so
-                // make sure that doesn't happen.
-                if (closed) {
-                    return;
+        protected void doClose() {
+            try {
+                getEndpoint().getHandler().release(this);
+            } catch (Throwable e) {
+                ExceptionUtils.handleThrowable(e);
+                if (log.isDebugEnabled()) {
+                    log.error(sm.getString("endpoint.debug.handlerRelease"), 
e);
                 }
-                closed = true;
+            }
+            socketBufferHandler = SocketBufferHandler.EMPTY;
+            nonBlockingWriteBuffer.clear();
+            synchronized (closed) {
                 if (sslOutputBuffer != null) {
                     ByteBufferUtils.cleanDirectBuffer(sslOutputBuffer);
                 }
@@ -2361,14 +2357,6 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
 
 
         @Override
-        public boolean isClosed() {
-            synchronized (closedLock) {
-                return closed;
-            }
-        }
-
-
-        @Override
         protected void writeBlockingDirect(ByteBuffer from) throws IOException 
{
             if (from.isDirect()) {
                 super.writeBlockingDirect(from);
@@ -2421,7 +2409,7 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
 
         @Override
         protected void doWrite(boolean block, ByteBuffer from) throws 
IOException {
-            if (closed) {
+            if (isClosed()) {
                 throw new IOException(sm.getString("socket.apr.closed", 
getSocket()));
             }
 
@@ -2521,8 +2509,8 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
         @Override
         public void registerReadInterest() {
             // Make sure an already closed socket is not added to the poller
-            synchronized (closedLock) {
-                if (closed) {
+            synchronized (closed) {
+                if (isClosed()) {
                     return;
                 }
                 Poller p = ((AprEndpoint) getEndpoint()).getPoller();
@@ -2536,8 +2524,8 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
         @Override
         public void registerWriteInterest() {
             // Make sure an already closed socket is not added to the poller
-            synchronized (closedLock) {
-                if (closed) {
+            synchronized (closed) {
+                if (isClosed()) {
                     return;
                 }
                 ((AprEndpoint) getEndpoint()).getPoller().add(
@@ -2561,7 +2549,7 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
 
         @Override
         protected void populateRemoteAddr() {
-            if (closed) {
+            if (isClosed()) {
                 return;
             }
             try {
@@ -2576,7 +2564,7 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
 
         @Override
         protected void populateRemoteHost() {
-            if (closed) {
+            if (isClosed()) {
                 return;
             }
             try {
@@ -2594,7 +2582,7 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
 
         @Override
         protected void populateRemotePort() {
-            if (closed) {
+            if (isClosed()) {
                 return;
             }
             try {
@@ -2610,7 +2598,7 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
 
         @Override
         protected void populateLocalName() {
-            if (closed) {
+            if (isClosed()) {
                 return;
             }
             try {
@@ -2625,7 +2613,7 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
 
         @Override
         protected void populateLocalAddr() {
-            if (closed) {
+            if (isClosed()) {
                 return;
             }
             try {
@@ -2640,7 +2628,7 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
 
         @Override
         protected void populateLocalPort() {
-            if (closed) {
+            if (isClosed()) {
                 return;
             }
             try {
@@ -2724,8 +2712,8 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
         }
 
         String getSSLInfoS(int id) {
-            synchronized (closedLock) {
-                if (closed) {
+            synchronized (closed) {
+                if (isClosed()) {
                     return null;
                 }
                 try {
@@ -2737,8 +2725,8 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
         }
 
         int getSSLInfoI(int id) {
-            synchronized (closedLock) {
-                if (closed) {
+            synchronized (closed) {
+                if (isClosed()) {
                     return 0;
                 }
                 try {
@@ -2750,8 +2738,8 @@ public class AprEndpoint extends 
AbstractEndpoint<Long,Long> implements SNICallB
         }
 
         byte[] getSSLInfoB(int id) {
-            synchronized (closedLock) {
-                if (closed) {
+            synchronized (closed) {
+                if (isClosed()) {
                     return null;
                 }
                 try {
diff --git a/java/org/apache/tomcat/util/net/Nio2Channel.java 
b/java/org/apache/tomcat/util/net/Nio2Channel.java
index 40eb9fb..36c5b99 100644
--- a/java/org/apache/tomcat/util/net/Nio2Channel.java
+++ b/java/org/apache/tomcat/util/net/Nio2Channel.java
@@ -20,6 +20,7 @@ import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.channels.AsynchronousByteChannel;
 import java.nio.channels.AsynchronousSocketChannel;
+import java.nio.channels.ClosedChannelException;
 import java.nio.channels.CompletionHandler;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
@@ -135,7 +136,7 @@ public class Nio2Channel implements AsynchronousByteChannel 
{
 
     @Override
     public String toString() {
-        return super.toString()+":"+this.sc.toString();
+        return super.toString() + ":" + sc.toString();
     }
 
     @Override
@@ -213,7 +214,6 @@ public class Nio2Channel implements AsynchronousByteChannel 
{
         return DONE;
     }
 
-
     private ApplicationBufferHandler appReadBufHandler;
     public void setAppReadBufHandler(ApplicationBufferHandler handler) {
         this.appReadBufHandler = handler;
@@ -221,4 +221,85 @@ public class Nio2Channel implements 
AsynchronousByteChannel {
     protected ApplicationBufferHandler getAppReadBufHandler() {
         return appReadBufHandler;
     }
+
+    private static final Future<Integer> DONE_INT = new Future<Integer>() {
+        @Override
+        public boolean cancel(boolean mayInterruptIfRunning) {
+            return false;
+        }
+        @Override
+        public boolean isCancelled() {
+            return false;
+        }
+        @Override
+        public boolean isDone() {
+            return true;
+        }
+        @Override
+        public Integer get() throws InterruptedException,
+                ExecutionException {
+            return Integer.valueOf(-1);
+        }
+        @Override
+        public Integer get(long timeout, TimeUnit unit)
+                throws InterruptedException, ExecutionException,
+                TimeoutException {
+            return Integer.valueOf(-1);
+        }
+    };
+
+    static final Nio2Channel CLOSED_NIO2_CHANNEL = new ClosedNio2Channel();
+    public static class ClosedNio2Channel extends Nio2Channel {
+        public ClosedNio2Channel() {
+            super(null);
+        }
+        @Override
+        public void close() throws IOException {
+        }
+        @Override
+        public boolean isOpen() {
+            return false;
+        }
+        @Override
+        public void reset(AsynchronousSocketChannel channel, 
SocketWrapperBase<Nio2Channel> socket) throws IOException {
+        }
+        @Override
+        public void free() {
+        }
+        @Override
+        public Future<Integer> read(ByteBuffer dst) {
+            return DONE_INT;
+        }
+        @Override
+        public <A> void read(ByteBuffer dst,
+                long timeout, TimeUnit unit, A attachment,
+                CompletionHandler<Integer, ? super A> handler) {
+            handler.failed(new ClosedChannelException(), attachment);
+        }
+        @Override
+        public <A> void read(ByteBuffer[] dsts,
+                int offset, int length, long timeout, TimeUnit unit,
+                A attachment, CompletionHandler<Long,? super A> handler) {
+            handler.failed(new ClosedChannelException(), attachment);
+        }
+        @Override
+        public Future<Integer> write(ByteBuffer src) {
+            return DONE_INT;
+        }
+        @Override
+        public <A> void write(ByteBuffer src, long timeout, TimeUnit unit, A 
attachment,
+                CompletionHandler<Integer, ? super A> handler) {
+            handler.failed(new ClosedChannelException(), attachment);
+        }
+        @Override
+        public <A> void write(ByteBuffer[] srcs, int offset, int length,
+                long timeout, TimeUnit unit, A attachment,
+                CompletionHandler<Long,? super A> handler) {
+            handler.failed(new ClosedChannelException(), attachment);
+        }
+        @Override
+        public String toString() {
+            return "Closed Nio2Channel";
+        }
+    }
 }
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java 
b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 50eb996..ee6a9a1 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -459,6 +459,8 @@ public class Nio2Endpoint extends 
AbstractJsseEndpoint<Nio2Channel,AsynchronousS
 
     public static class Nio2SocketWrapper extends 
SocketWrapperBase<Nio2Channel> {
 
+        private final SynchronizedStack<Nio2Channel> nioChannels;
+
         private SendfileData sendfileData = null;
 
         private final CompletionHandler<Integer, ByteBuffer> 
readCompletionHandler;
@@ -470,8 +472,6 @@ public class Nio2Endpoint extends 
AbstractJsseEndpoint<Nio2Channel,AsynchronousS
         private boolean writeInterest = false; // Guarded by 
writeCompletionHandler
         private boolean writeNotify = false;
 
-        private volatile boolean closed = false;
-
         private CompletionHandler<Integer, SendfileData> sendfileHandler
             = new CompletionHandler<Integer, SendfileData>() {
 
@@ -555,6 +555,7 @@ public class Nio2Endpoint extends 
AbstractJsseEndpoint<Nio2Channel,AsynchronousS
 
         public Nio2SocketWrapper(Nio2Channel channel, final Nio2Endpoint 
endpoint) {
             super(channel, endpoint);
+            nioChannels = endpoint.nioChannels;
             socketBufferHandler = channel.getBufHandler();
 
             this.readCompletionHandler = new CompletionHandler<Integer, 
ByteBuffer>() {
@@ -894,7 +895,7 @@ public class Nio2Endpoint extends 
AbstractJsseEndpoint<Nio2Channel,AsynchronousS
 
 
         @Override
-        public void close() {
+        protected void doClose() {
             if (log.isDebugEnabled()) {
                 log.debug("Calling [" + getEndpoint() + "].closeSocket([" + 
this + "])", new Exception());
             }
@@ -908,19 +909,25 @@ public class Nio2Endpoint extends 
AbstractJsseEndpoint<Nio2Channel,AsynchronousS
             }
             try {
                 synchronized (getSocket()) {
-                    if (!closed) {
-                        closed = true;
-                        getEndpoint().countDownConnection();
-                    }
+                    getEndpoint().countDownConnection();
                     if (getSocket().isOpen()) {
                         getSocket().close(true);
                     }
+                    socketBufferHandler = SocketBufferHandler.EMPTY;
+                    nonBlockingWriteBuffer.clear();
+                    if (getEndpoint().running && !getEndpoint().paused) {
+                        if (nioChannels == null || 
!nioChannels.push(getSocket())) {
+                            getSocket().free();
+                        }
+                    }
                 }
             } catch (Throwable e) {
                 ExceptionUtils.handleThrowable(e);
                 if (log.isDebugEnabled()) {
                     log.error(sm.getString("endpoint.debug.channelCloseFail"), 
e);
                 }
+            } finally {
+                reset(Nio2Channel.CLOSED_NIO2_CHANNEL);
             }
             try {
                 SendfileData data = getSendfileData();
@@ -936,12 +943,6 @@ public class Nio2Endpoint extends 
AbstractJsseEndpoint<Nio2Channel,AsynchronousS
         }
 
         @Override
-        public boolean isClosed() {
-            return closed;
-        }
-
-
-        @Override
         public boolean hasAsyncIO() {
             return getEndpoint().getUseAsyncIO();
         }
@@ -1630,21 +1631,11 @@ public class Nio2Endpoint extends 
AbstractJsseEndpoint<Nio2Channel,AsynchronousS
                     if (state == SocketState.CLOSED) {
                         // Close socket and pool
                         socketWrapper.close();
-                        if (running && !paused) {
-                            if (nioChannels == null || 
!nioChannels.push(socketWrapper.getSocket())) {
-                                socketWrapper.getSocket().free();
-                            }
-                        }
                     } else if (state == SocketState.UPGRADING) {
                         launch = true;
                     }
                 } else if (handshake == -1 ) {
                     socketWrapper.close();
-                    if (running && !paused) {
-                        if (nioChannels == null || 
!nioChannels.push(socketWrapper.getSocket())) {
-                            socketWrapper.getSocket().free();
-                        }
-                    }
                 }
             } catch (VirtualMachineError vme) {
                 ExceptionUtils.handleThrowable(vme);
diff --git a/java/org/apache/tomcat/util/net/NioChannel.java 
b/java/org/apache/tomcat/util/net/NioChannel.java
index 01222e6..9489b72 100644
--- a/java/org/apache/tomcat/util/net/NioChannel.java
+++ b/java/org/apache/tomcat/util/net/NioChannel.java
@@ -102,8 +102,8 @@ public class NioChannel implements ByteChannel, 
ScatteringByteChannel, Gathering
      */
     @Override
     public void close() throws IOException {
-        getIOChannel().socket().close();
-        getIOChannel().close();
+        sc.socket().close();
+        sc.close();
     }
 
     /**
@@ -205,13 +205,13 @@ public class NioChannel implements ByteChannel, 
ScatteringByteChannel, Gathering
         return 0;
     }
 
-    public void setIOChannel(SocketChannel IOChannel) {
-        this.sc = IOChannel;
+    public void setIOChannel(SocketChannel sc) {
+        this.sc = sc;
     }
 
     @Override
     public String toString() {
-        return super.toString()+":"+this.sc.toString();
+        return super.toString() + ":" + sc.toString();
     }
 
     public int getOutboundRemaining() {
@@ -255,4 +255,46 @@ public class NioChannel implements ByteChannel, 
ScatteringByteChannel, Gathering
         return appReadBufHandler;
     }
 
+    static final NioChannel CLOSED_NIO_CHANNEL = new ClosedNioChannel();
+    public static class ClosedNioChannel extends NioChannel {
+        public ClosedNioChannel() {
+            super(null, null);
+        }
+        @Override
+        public void close() throws IOException {
+        }
+        @Override
+        public boolean isOpen() {
+            return false;
+        }
+        @Override
+        public void reset() throws IOException {
+        }
+        @Override
+        public void free() {
+        }
+        @Override
+        public int read(ByteBuffer dst) throws IOException {
+            return -1;
+        }
+        @Override
+        public long read(ByteBuffer[] dsts, int offset, int length)
+                throws IOException {
+            return -1L;
+        }
+        @Override
+        public int write(ByteBuffer src) throws IOException {
+            checkInterruptStatus();
+            return -1;
+        }
+        @Override
+        public long write(ByteBuffer[] srcs, int offset, int length)
+                throws IOException {
+            return -1L;
+        }
+        @Override
+        public String toString() {
+            return "Closed NioChannel";
+        }
+    }
 }
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java 
b/java/org/apache/tomcat/util/net/NioEndpoint.java
index 8449bf0..f31a508 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -466,31 +466,6 @@ public class NioEndpoint extends 
AbstractJsseEndpoint<NioChannel,SocketChannel>
         return new SocketProcessor(socketWrapper, event);
     }
 
-
-    private void close(NioChannel socket, SelectionKey key) {
-        try {
-            Poller poller = this.poller;
-            if (poller != null && poller.cancelledKey(key) != null) {
-                // SocketWrapper (attachment) was removed from the
-                // key - recycle the key. This can only happen once
-                // per attempted closure so it is used to determine
-                // whether or not to return the key to the cache.
-                // We do NOT want to do this more than once - see BZ
-                // 57340 / 57943.
-                if (log.isDebugEnabled()) {
-                    log.debug("Socket: [" + socket + "] closed");
-                }
-                if (running && !paused) {
-                    if (nioChannels == null || !nioChannels.push(socket)) {
-                        socket.free();
-                    }
-                }
-            }
-        } catch (Exception x) {
-            log.error(sm.getString("endpoint.err.close"), x);
-        }
-    }
-
     // ----------------------------------------------------- Poller Inner 
Classes
 
     /**
@@ -886,7 +861,8 @@ public class NioEndpoint extends 
AbstractJsseEndpoint<NioChannel,SocketChannel>
                             if (log.isDebugEnabled()) {
                                 log.debug("Send file connection is being 
closed");
                             }
-                            close(sc, sk);
+                            poller.cancelledKey(sk);
+                            socketWrapper.close();
                             break;
                         }
                         case PIPELINED: {
@@ -894,7 +870,8 @@ public class NioEndpoint extends 
AbstractJsseEndpoint<NioChannel,SocketChannel>
                                 log.debug("Connection is keep alive, 
processing pipe-lined data");
                             }
                             if (!processSocket(socketWrapper, 
SocketEvent.OPEN_READ, true)) {
-                                close(sc, sk);
+                                poller.cancelledKey(sk);
+                                socketWrapper.close();
                             }
                             break;
                         }
@@ -924,13 +901,15 @@ public class NioEndpoint extends 
AbstractJsseEndpoint<NioChannel,SocketChannel>
                     log.debug("Unable to complete sendfile request:", e);
                 }
                 if (!calledByProcessor && sc != null) {
-                    close(sc, sk);
+                    poller.cancelledKey(sk);
+                    socketWrapper.close();
                 }
                 return SendfileState.ERROR;
             } catch (Throwable t) {
                 log.error(sm.getString("endpoint.sendfile.error"), t);
                 if (!calledByProcessor && sc != null) {
-                    close(sc, sk);
+                    poller.cancelledKey(sk);
+                    socketWrapper.close();
                 }
                 return SendfileState.ERROR;
             }
@@ -1036,6 +1015,7 @@ public class NioEndpoint extends 
AbstractJsseEndpoint<NioChannel,SocketChannel>
     public static class NioSocketWrapper extends SocketWrapperBase<NioChannel> 
{
 
         private final NioSelectorPool pool;
+        private final SynchronizedStack<NioChannel> nioChannels;
 
         private Poller poller = null;
         private int interestOps = 0;
@@ -1044,12 +1024,12 @@ public class NioEndpoint extends 
AbstractJsseEndpoint<NioChannel,SocketChannel>
         private volatile SendfileData sendfileData = null;
         private volatile long lastRead = System.currentTimeMillis();
         private volatile long lastWrite = lastRead;
-        private volatile boolean closed = false;
 
         public NioSocketWrapper(NioChannel channel, NioEndpoint endpoint) {
             super(channel, endpoint);
             pool = endpoint.getSelectorPool();
             socketBufferHandler = channel.getBufHandler();
+            nioChannels = endpoint.nioChannels;
         }
 
         public Poller getPoller() { return poller; }
@@ -1184,7 +1164,7 @@ public class NioEndpoint extends 
AbstractJsseEndpoint<NioChannel,SocketChannel>
 
 
         @Override
-        public void close() {
+        protected void doClose() {
             if (log.isDebugEnabled()) {
                 log.debug("Calling [" + getEndpoint() + "].closeSocket([" + 
this + "])", new Exception());
             }
@@ -1198,19 +1178,25 @@ public class NioEndpoint extends 
AbstractJsseEndpoint<NioChannel,SocketChannel>
             }
             try {
                 synchronized (getSocket()) {
-                    if (!closed) {
-                        closed = true;
-                        getEndpoint().countDownConnection();
-                    }
+                    getEndpoint().countDownConnection();
                     if (getSocket().isOpen()) {
                         getSocket().close(true);
                     }
+                    socketBufferHandler = SocketBufferHandler.EMPTY;
+                    nonBlockingWriteBuffer.clear();
+                    if (getEndpoint().running && !getEndpoint().paused) {
+                        if (nioChannels == null || 
!nioChannels.push(getSocket())) {
+                            getSocket().free();
+                        }
+                    }
                 }
             } catch (Throwable e) {
                 ExceptionUtils.handleThrowable(e);
                 if (log.isDebugEnabled()) {
                     log.error(sm.getString("endpoint.debug.channelCloseFail"), 
e);
                 }
+            } finally {
+                reset(NioChannel.CLOSED_NIO_CHANNEL);
             }
             try {
                 SendfileData data = getSendfileData();
@@ -1225,13 +1211,6 @@ public class NioEndpoint extends 
AbstractJsseEndpoint<NioChannel,SocketChannel>
             }
         }
 
-
-        @Override
-        public boolean isClosed() {
-            return closed;
-        }
-
-
         private int fillReadBuffer(boolean block) throws IOException {
             socketBufferHandler.configureReadBufferForWrite();
             return fillReadBuffer(block, socketBufferHandler.getReadBuffer());
@@ -1540,6 +1519,11 @@ public class NioEndpoint extends 
AbstractJsseEndpoint<NioChannel,SocketChannel>
         protected void doRun() {
             NioChannel socket = socketWrapper.getSocket();
             SelectionKey key = 
socket.getIOChannel().keyFor(socket.getSocketWrapper().getPoller().getSelector());
+            Poller poller = NioEndpoint.this.poller;
+            if (poller == null) {
+                socketWrapper.close();
+                return;
+            }
 
             try {
                 int handshake = -1;
@@ -1582,10 +1566,12 @@ public class NioEndpoint extends 
AbstractJsseEndpoint<NioChannel,SocketChannel>
                         state = getHandler().process(socketWrapper, event);
                     }
                     if (state == SocketState.CLOSED) {
-                        close(socket, key);
+                        poller.cancelledKey(key);
+                        socketWrapper.close();
                     }
                 } else if (handshake == -1 ) {
-                    close(socket, key);
+                    poller.cancelledKey(key);
+                    socketWrapper.close();
                 } else if (handshake == SelectionKey.OP_READ){
                     socketWrapper.registerReadInterest();
                 } else if (handshake == SelectionKey.OP_WRITE){
diff --git a/java/org/apache/tomcat/util/net/SocketBufferHandler.java 
b/java/org/apache/tomcat/util/net/SocketBufferHandler.java
index 134ee1b..89fa86e 100644
--- a/java/org/apache/tomcat/util/net/SocketBufferHandler.java
+++ b/java/org/apache/tomcat/util/net/SocketBufferHandler.java
@@ -22,6 +22,12 @@ import org.apache.tomcat.util.buf.ByteBufferUtils;
 
 public class SocketBufferHandler {
 
+    static SocketBufferHandler EMPTY = new SocketBufferHandler(0, 0, false) {
+        @Override
+        public void expand(int newSize) {
+        }
+    };
+
     private volatile boolean readBufferConfiguredForWrite = true;
     private volatile ByteBuffer readBuffer;
 
diff --git a/java/org/apache/tomcat/util/net/SocketWrapperBase.java 
b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
index b42c603..01502cb 100644
--- a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
+++ b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
@@ -28,6 +28,7 @@ import java.util.concurrent.Executor;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock;
@@ -43,9 +44,11 @@ public abstract class SocketWrapperBase<E> {
 
     protected static final StringManager sm = 
StringManager.getManager(SocketWrapperBase.class);
 
-    private final E socket;
+    private E socket;
     private final AbstractEndpoint<E,?> endpoint;
 
+    protected final AtomicBoolean closed = new AtomicBoolean(false);
+
     // Volatile because I/O and setting the timeout values occurs on a 
different
     // thread to the thread checking the timeout.
     private volatile long readTimeout = -1;
@@ -124,6 +127,10 @@ public abstract class SocketWrapperBase<E> {
         return socket;
     }
 
+    protected void reset(E closedSocket) {
+        socket = closedSocket;
+    }
+
     protected AbstractEndpoint<E,?> getEndpoint() {
         return endpoint;
     }
@@ -379,8 +386,27 @@ public abstract class SocketWrapperBase<E> {
     }
 
 
-    public abstract void close() throws IOException;
-    public abstract boolean isClosed();
+    /**
+     * Close the socket wrapper.
+     */
+    public void close() {
+        if (closed.compareAndSet(false, true)) {
+            doClose();
+        }
+    }
+
+    /**
+     * Perform the actual close. The closed atomic boolean guarantees this will
+     * be called only once per wrapper.
+     */
+    protected abstract void doClose();
+
+    /**
+     * @return true if the wrapper has been closed
+     */
+    public boolean isClosed() {
+        return closed.get();
+    }
 
 
     /**
diff --git a/java/org/apache/tomcat/util/net/WriteBuffer.java 
b/java/org/apache/tomcat/util/net/WriteBuffer.java
index 4a1d2c5..e4d28cb 100644
--- a/java/org/apache/tomcat/util/net/WriteBuffer.java
+++ b/java/org/apache/tomcat/util/net/WriteBuffer.java
@@ -43,6 +43,9 @@ public class WriteBuffer {
         this.bufferSize = bufferSize;
     }
 
+    void clear() {
+        buffers.clear();
+    }
 
     void add(byte[] buf, int offset, int length) {
         ByteBufferHolder holder = getByteBufferHolder(length);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 459c293..0a0d1e3 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -112,6 +112,18 @@
         Refactor async IO implementation to the <code>SocketWrapperBase</code>.
         (remm)
       </scode>
+      <update>
+        Refactor <code>SocketWrapperBase</code> close using an atomic boolean
+        and a <code>doClose</code> method that subclasses will implement, with
+        a guarantee that it will be run only once. (remm)
+      </update>
+      <fix>
+        Decouple the socket wrapper, which is not recycled, from the NIOx
+        channel after close, and replace it with a dummy static object. (remm)
+      </fix>
+      <fix>
+        Clear buffers on socket wrapper close. (remm)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Other">


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

Reply via email to