2015-03-11 7:35 GMT+09:00 <ma...@apache.org>: > Author: markt > Date: Tue Mar 10 22:35:19 2015 > New Revision: 1665736 > > URL: http://svn.apache.org/r1665736 > Log: > Stop re-using the SocketWrapper > With the introduction of upgrade and non-blocking, I/O can occur on > non-container threads. This makes it near impossible to track whether a > SocketWrapper (== connection) is still referenced or not, making re-use a > risky proposition. > > Modified: > tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java > tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java > tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java > tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java > > Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=1665736&r1=1665735&r2=1665736&view=diff > > ============================================================================== > --- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java > (original) > +++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Tue Mar > 10 22:35:19 2015 > @@ -2332,7 +2332,6 @@ public class AprEndpoint extends Abstrac > if (state == Handler.SocketState.CLOSED) { > // Close socket and pool > closeSocket(socket.getSocket().longValue()); > - socket.reset(null, 1); > } else if (state == Handler.SocketState.LONG) { > if (socket.isAsync()) { > waitingRequests.add(socket); > @@ -2375,12 +2374,6 @@ public class AprEndpoint extends Abstrac > } > > > - @Override > - protected void resetSocketBufferHandler(Long socket) { > - socketBufferHandler.reset(); > - } > - > - > @Override > public int read(boolean block, byte[] b, int off, int len) > throws IOException { > > Modified: tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java?rev=1665736&r1=1665735&r2=1665736&view=diff > > ============================================================================== > --- tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java > (original) > +++ tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Tue Mar > 10 22:35:19 2015 > @@ -118,11 +118,6 @@ public class Nio2Endpoint extends Abstra > private SynchronizedStack<SocketProcessor> processorCache; > > /** > - * Cache for socket wrapper objects > - */ > - private SynchronizedStack<Nio2SocketWrapper> socketWrapperCache; > - > - /** > * Bytebuffer cache, each channel holds a set of buffers (two, except > for SSL holds four) > */ > private SynchronizedStack<Nio2Channel> nioChannels; > @@ -234,7 +229,6 @@ public class Nio2Endpoint extends Abstra > > protected void releaseCaches() { > if (useCaches) { > - this.socketWrapperCache.clear(); > this.nioChannels.clear(); > this.processorCache.clear(); > } > @@ -337,8 +331,6 @@ public class Nio2Endpoint extends Abstra > if (useCaches) { > processorCache = new > SynchronizedStack<>(SynchronizedStack.DEFAULT_SIZE, > socketProperties.getProcessorCache()); > - socketWrapperCache = new > SynchronizedStack<>(SynchronizedStack.DEFAULT_SIZE, > - socketProperties.getSocketWrapperCache()); > nioChannels = new > SynchronizedStack<>(SynchronizedStack.DEFAULT_SIZE, > socketProperties.getBufferPool()); > } > @@ -395,7 +387,6 @@ public class Nio2Endpoint extends Abstra > } > }); > if (useCaches) { > - socketWrapperCache.clear(); > nioChannels.clear(); > processorCache.clear(); > } > @@ -506,12 +497,10 @@ public class Nio2Endpoint extends Abstra > ((SecureNio2Channel) channel).setSSLEngine(engine); > } > } > - Nio2SocketWrapper socketWrapper = (useCaches) ? > socketWrapperCache.pop() : null; > - if (socketWrapper == null) { > - socketWrapper = new Nio2SocketWrapper(channel, this); > - } > + Nio2SocketWrapper socketWrapper = new > Nio2SocketWrapper(channel, this); > channel.reset(socket, socketWrapper); > - socketWrapper.reset(channel, > getSocketProperties().getSoTimeout()); > + > socketWrapper.setReadTimeout(getSocketProperties().getSoTimeout()); > + > socketWrapper.setWriteTimeout(getSocketProperties().getSoTimeout()); > > socketWrapper.setKeepAliveLeft(Nio2Endpoint.this.getMaxKeepAliveRequests()); > socketWrapper.setSecure(isSSLEnabled()); > socketWrapper.setReadTimeout(getSoTimeout()); > @@ -621,7 +610,6 @@ public class Nio2Endpoint extends Abstra > } > } catch (Exception ignore) { > } > - nio2Socket.reset(null, -1); > countDownConnection(); > } catch (Throwable e) { > ExceptionUtils.handleThrowable(e); > @@ -934,26 +922,6 @@ public class Nio2Endpoint extends Abstra > return false; > } > > - @Override > - public void reset(Nio2Channel channel, long soTimeout) { > - if (log.isDebugEnabled()) { > - log.debug("Calling [" + this + "].reset([" + channel + > "],[" + soTimeout + "])", > - new Exception()); > - } > - super.reset(channel, soTimeout); > - sendfileData = null; > - } > - > - > - @Override > - protected void resetSocketBufferHandler(Nio2Channel socket) { > - if (socket == null) { > - socketBufferHandler = null; > - } else { > - socketBufferHandler = socket.getBufHandler(); > - } > - } > - > > public void setSendfileData(SendfileData sf) { this.sendfileData > = sf; } > public SendfileData getSendfileData() { return this.sendfileData; > } > @@ -1672,7 +1640,6 @@ public class Nio2Endpoint extends Abstra > closeSocket(socket); > if (useCaches && running && !paused) { > nioChannels.push(socket.getSocket()); > - > socketWrapperCache.push((Nio2SocketWrapper) socket); > } > } else if (state == SocketState.UPGRADING) { > socket.setKeptAlive(true); > @@ -1682,7 +1649,6 @@ public class Nio2Endpoint extends Abstra > closeSocket(socket); > if (useCaches && running && !paused) { > nioChannels.push(socket.getSocket()); > - socketWrapperCache.push(((Nio2SocketWrapper) > socket)); > } > } > } catch (OutOfMemoryError oom) { > > Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1665736&r1=1665735&r2=1665736&view=diff > > ============================================================================== > --- tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java > (original) > +++ tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Tue Mar > 10 22:35:19 2015 > @@ -125,11 +125,6 @@ public class NioEndpoint extends Abstrac > private SynchronizedStack<SocketProcessor> processorCache; > > /** > - * Cache for key attachment objects > - */ > - private SynchronizedStack<NioSocketWrapper> keyCache; > - > - /** > * Cache for poller events > */ > private SynchronizedStack<PollerEvent> eventCache; > @@ -290,7 +285,6 @@ public class NioEndpoint extends Abstrac > } > > protected void releaseCaches() { > - this.keyCache.clear(); > this.nioChannels.clear(); > this.processorCache.clear(); > if ( handler != null ) handler.recycle(); > @@ -393,8 +387,6 @@ public class NioEndpoint extends Abstrac > > processorCache = new > SynchronizedStack<>(SynchronizedStack.DEFAULT_SIZE, > socketProperties.getProcessorCache()); > - keyCache = new > SynchronizedStack<>(SynchronizedStack.DEFAULT_SIZE, > - socketProperties.getKeyCache()); > eventCache = new > SynchronizedStack<>(SynchronizedStack.DEFAULT_SIZE, > socketProperties.getEventCache()); > nioChannels = new > SynchronizedStack<>(SynchronizedStack.DEFAULT_SIZE, > @@ -445,7 +437,6 @@ public class NioEndpoint extends Abstrac > } > shutdownExecutor(); > eventCache.clear(); > - keyCache.clear(); > nioChannels.clear(); > processorCache.clear(); > } > @@ -900,9 +891,9 @@ public class NioEndpoint extends Abstrac > */ > public void register(final NioChannel socket) { > socket.setPoller(this); > - NioSocketWrapper key = keyCache.pop(); > - final NioSocketWrapper ka = key!=null?key:new > NioSocketWrapper(socket, NioEndpoint.this); > - ka.reset(this,socket,getSocketProperties().getSoTimeout()); > + NioSocketWrapper ka = new NioSocketWrapper(socket, > NioEndpoint.this); > + ka.setReadTimeout(getSocketProperties().getSoTimeout()); > + ka.setWriteTimeout(getSocketProperties().getSoTimeout()); > > ka.setKeepAliveLeft(NioEndpoint.this.getMaxKeepAliveRequests()); > ka.setSecure(isSSLEnabled()); > ka.setReadTimeout(getSoTimeout()); > @@ -950,8 +941,7 @@ public class NioEndpoint extends Abstrac > } > } catch (Exception ignore) { > } > - if (ka!=null) { > - ka.reset(); > + if (ka != null) { > countDownConnection(); > } > } catch (Throwable e) { > @@ -1307,48 +1297,6 @@ public class NioEndpoint extends Abstrac > pool = endpoint.getSelectorPool(); > } > > - public void reset(Poller poller, NioChannel channel, long > soTimeout) { > - super.reset(channel, soTimeout); > - > - interestOps = 0; > - this.poller = poller; > - sendfileData = null; > - if (readLatch != null) { > - try { > - for (int i = 0; i < (int) readLatch.getCount(); i++) { > - readLatch.countDown(); > - } > - } catch (Exception ignore) { > - } > - } > - readLatch = null; > - sendfileData = null; > - if (writeLatch != null) { > - try { > - for (int i = 0; i < (int) writeLatch.getCount(); i++) > { > - writeLatch.countDown(); > - } > - } catch (Exception ignore) { > - } > - } > - writeLatch = null; > - } > - > - > - @Override > - protected void resetSocketBufferHandler(NioChannel socket) { > - if (socket == null) { > - socketBufferHandler = null; > - } else { > - socketBufferHandler = socket.getBufHandler(); > - } > - } > - > - > - public void reset() { > - reset(null,null,-1); > - } > - > public Poller getPoller() { return poller;} > public void setPoller(Poller poller){this.poller = poller;} > public int interestOps() { return interestOps;} > @@ -1731,9 +1679,6 @@ public class NioEndpoint extends Abstrac > nioChannels.push(socket); > } > socket = null; > - if (running && !paused) { > - keyCache.push(ka); > - } > } > ka = null; > } catch (Exception x) { > @@ -1748,9 +1693,6 @@ public class NioEndpoint extends Abstrac > nioChannels.push(socket); > } > socket = null; > - if (running && !paused) { > - keyCache.push(ka); > - } > ka = null; > } else { > ka.getPoller().add(socket,handshake); > > Modified: > tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java?rev=1665736&r1=1665735&r2=1665736&view=diff > > ============================================================================== > --- tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java > (original) > +++ tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java > Tue Mar 10 22:35:19 2015 > @@ -326,30 +326,6 @@ public abstract class SocketWrapperBase< > } > } > > - public void reset(E socket, long soTimeout) { > - async = false; > - blockingStatus = true; > - dispatches.clear(); > - error = null; > - keepAliveLeft = 100; > - lastRead = System.currentTimeMillis(); > - lastWrite = lastRead; > - lastAsyncStart = 0; > - asyncTimeout = -1; > - localAddr = null; > - localName = null; > - localPort = -1; > - remoteAddr = null; > - remoteHost = null; > - remotePort = -1; > - this.socket = socket; > - this.readTimeout = soTimeout; > - this.writeTimeout = soTimeout; > - upgraded = false; > - resetSocketBufferHandler(socket); > - } > - > - protected abstract void resetSocketBufferHandler(E socket); > > /** > * Overridden for debug purposes. No guarantees are made about the > format of > >
I think this fix causes NPE. Do we need initialization of poller and socketBufferHandler in SocketWrapper? > -- > Keiichi.Fujino > <dev-h...@tomcat.apache.org>
