Skip to content

Commit

Permalink
Fix various errors when stopping the APR connector via JMX (may occur…
Browse files Browse the repository at this point in the history
… in other scenarios)

- prevent a crash by ensuring all connections are closed via the SocketWrapper
- prevent a memory leak by releasing any Processor associated with a closed connection
Adds release(SocketWrapper) to the Handler interface and adds an implementation to the base AbstractProtocol.AbstractConnectionHandler (replacing the impl. in vaious sub-classes)

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1682240 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
markt-asf committed May 28, 2015
1 parent 0df688f commit a098125
Show file tree
Hide file tree
Showing 9 changed files with 33 additions and 69 deletions.
17 changes: 17 additions & 0 deletions java/org/apache/coyote/AbstractProtocol.java
Expand Up @@ -868,6 +868,23 @@ public void release(SocketWrapperBase<S> socket, Processor processor, boolean ad
} }




/**
* Expected to be used by the Endpoint to release resources on socket
* close, errors etc.
*/
@Override
public void release(SocketWrapperBase<S> socketWrapper) {
S socket = socketWrapper.getSocket();
if (socket != null) {
Processor processor = connections.remove(socket);
if (processor != null) {
processor.recycle();
recycledProcessors.push(processor);
}
}
}


protected abstract Processor createUpgradeProcessor( protected abstract Processor createUpgradeProcessor(
SocketWrapperBase<?> socket, ByteBuffer leftoverInput, SocketWrapperBase<?> socket, ByteBuffer leftoverInput,
HttpUpgradeHandler httpUpgradeHandler) throws IOException; HttpUpgradeHandler httpUpgradeHandler) throws IOException;
Expand Down
16 changes: 0 additions & 16 deletions java/org/apache/coyote/ajp/AjpNio2Protocol.java
Expand Up @@ -16,13 +16,11 @@
*/ */
package org.apache.coyote.ajp; package org.apache.coyote.ajp;


import org.apache.coyote.Processor;
import org.apache.juli.logging.Log; import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory; import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.net.Nio2Channel; import org.apache.tomcat.util.net.Nio2Channel;
import org.apache.tomcat.util.net.Nio2Endpoint; import org.apache.tomcat.util.net.Nio2Endpoint;
import org.apache.tomcat.util.net.Nio2Endpoint.Handler; import org.apache.tomcat.util.net.Nio2Endpoint.Handler;
import org.apache.tomcat.util.net.SocketWrapperBase;




/** /**
Expand Down Expand Up @@ -71,20 +69,6 @@ protected Log getLog() {
} }




/**
* Expected to be used by the Poller to release resources on socket
* close, errors etc.
*/
@Override
public void release(SocketWrapperBase<Nio2Channel> socket) {
Processor processor = connections.remove(socket.getSocket());
if (processor != null) {
processor.recycle();
recycledProcessors.push(processor);
}
}


@Override @Override
public void closeAll() { public void closeAll() {
for (Nio2Channel channel : connections.keySet()) { for (Nio2Channel channel : connections.keySet()) {
Expand Down
15 changes: 0 additions & 15 deletions java/org/apache/coyote/ajp/AjpNioProtocol.java
Expand Up @@ -25,7 +25,6 @@
import org.apache.tomcat.util.net.NioChannel; import org.apache.tomcat.util.net.NioChannel;
import org.apache.tomcat.util.net.NioEndpoint; import org.apache.tomcat.util.net.NioEndpoint;
import org.apache.tomcat.util.net.NioEndpoint.Handler; import org.apache.tomcat.util.net.NioEndpoint.Handler;
import org.apache.tomcat.util.net.SocketWrapperBase;


/** /**
* This the NIO based protocol handler implementation for AJP. * This the NIO based protocol handler implementation for AJP.
Expand Down Expand Up @@ -96,19 +95,5 @@ public void release(SocketChannel socket) {
log.debug(sm.getString("ajpnioprotocol.releaseEnd", log.debug(sm.getString("ajpnioprotocol.releaseEnd",
socket, Boolean.valueOf(released))); socket, Boolean.valueOf(released)));
} }


/**
* Expected to be used by the Poller to release resources on socket
* close, errors etc.
*/
@Override
public void release(SocketWrapperBase<NioChannel> socket) {
Processor processor = connections.remove(socket.getSocket());
if (processor != null) {
processor.recycle();
recycledProcessors.push(processor);
}
}
} }
} }
19 changes: 0 additions & 19 deletions java/org/apache/coyote/http11/Http11Nio2Protocol.java
Expand Up @@ -16,13 +16,11 @@
*/ */
package org.apache.coyote.http11; package org.apache.coyote.http11;


import org.apache.coyote.Processor;
import org.apache.juli.logging.Log; import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory; import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.net.Nio2Channel; import org.apache.tomcat.util.net.Nio2Channel;
import org.apache.tomcat.util.net.Nio2Endpoint; import org.apache.tomcat.util.net.Nio2Endpoint;
import org.apache.tomcat.util.net.Nio2Endpoint.Handler; import org.apache.tomcat.util.net.Nio2Endpoint.Handler;
import org.apache.tomcat.util.net.SocketWrapperBase;




/** /**
Expand Down Expand Up @@ -72,23 +70,6 @@ protected Log getLog() {
return log; return log;
} }


/**
* Expected to be used by the Endpoint to release resources on socket
* close, errors etc.
*/
@Override
public void release(SocketWrapperBase<Nio2Channel> socketWrapper) {
Nio2Channel socket = socketWrapper.getSocket();
if (socket != null) {
Processor processor = connections.remove(socket);
if (processor != null) {
processor.recycle();
recycledProcessors.push(processor);
}
}
}


@Override @Override
public void closeAll() { public void closeAll() {
for (Nio2Channel channel : connections.keySet()) { for (Nio2Channel channel : connections.keySet()) {
Expand Down
17 changes: 0 additions & 17 deletions java/org/apache/coyote/http11/Http11NioProtocol.java
Expand Up @@ -25,7 +25,6 @@
import org.apache.tomcat.util.net.NioChannel; import org.apache.tomcat.util.net.NioChannel;
import org.apache.tomcat.util.net.NioEndpoint; import org.apache.tomcat.util.net.NioEndpoint;
import org.apache.tomcat.util.net.NioEndpoint.Handler; import org.apache.tomcat.util.net.NioEndpoint.Handler;
import org.apache.tomcat.util.net.SocketWrapperBase;




/** /**
Expand Down Expand Up @@ -132,21 +131,5 @@ public void release(SocketChannel socket) {
if (log.isDebugEnabled()) if (log.isDebugEnabled())
log.debug("Done iterating through our connections to release a socket channel:"+socket +" released:"+released); log.debug("Done iterating through our connections to release a socket channel:"+socket +" released:"+released);
} }

/**
* Expected to be used by the Endpoint to release resources on socket
* close, errors etc.
*/
@Override
public void release(SocketWrapperBase<NioChannel> socketWrapper) {
NioChannel socket = socketWrapper.getSocket();
if (socket != null) {
Processor processor = connections.remove(socket);
if (processor != null) {
processor.recycle();
recycledProcessors.push(processor);
}
}
}
} }
} }
8 changes: 8 additions & 0 deletions java/org/apache/tomcat/util/net/AbstractEndpoint.java
Expand Up @@ -85,6 +85,14 @@ public SocketState process(SocketWrapperBase<S> socket,
public Object getGlobal(); public Object getGlobal();




/**
* Release any resources associated with the given SocketWrapper.
*
* @param socketWrapper The socketWrapper to release resources for
*/
public void release(SocketWrapperBase<S> socketWrapper);


/** /**
* Recycle resources associated with the handler. * Recycle resources associated with the handler.
*/ */
Expand Down
8 changes: 8 additions & 0 deletions java/org/apache/tomcat/util/net/AprEndpoint.java
Expand Up @@ -661,6 +661,14 @@ public void stopInternal() {
if (running) { if (running) {
running = false; running = false;
poller.stop(); poller.stop();
for (SocketWrapperBase<Long> socketWrapper : connections.values()) {
try {
socketWrapper.close();
handler.release(socketWrapper);
} catch (IOException e) {
// Ignore
}
}
getAsyncTimeout().stop(); getAsyncTimeout().stop();
for (AbstractEndpoint.Acceptor acceptor : acceptors) { for (AbstractEndpoint.Acceptor acceptor : acceptors) {
long waitLeft = 10000; long waitLeft = 10000;
Expand Down
1 change: 0 additions & 1 deletion java/org/apache/tomcat/util/net/Nio2Endpoint.java
Expand Up @@ -1571,7 +1571,6 @@ public void doClientAuth(SSLSupport sslSupport) {
* thread local fields. * thread local fields.
*/ */
public interface Handler extends AbstractEndpoint.Handler<Nio2Channel> { public interface Handler extends AbstractEndpoint.Handler<Nio2Channel> {
public void release(SocketWrapperBase<Nio2Channel> socket);
public void closeAll(); public void closeAll();
} }


Expand Down
1 change: 0 additions & 1 deletion java/org/apache/tomcat/util/net/NioEndpoint.java
Expand Up @@ -1455,7 +1455,6 @@ public <A> CompletionState write(ByteBuffer[] srcs, int offset,
* thread local fields. * thread local fields.
*/ */
public interface Handler extends AbstractEndpoint.Handler<NioChannel> { public interface Handler extends AbstractEndpoint.Handler<NioChannel> {
public void release(SocketWrapperBase<NioChannel> socket);
public void release(SocketChannel socket); public void release(SocketChannel socket);
} }


Expand Down

0 comments on commit a098125

Please sign in to comment.