This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push: new 010f735 Refactor to reduce JVM crashes on shutdown when sendfile is used 010f735 is described below commit 010f7354772e465b588f673249ca8d204b9abbb0 Author: Mark Thomas <ma...@apache.org> AuthorDate: Fri Nov 26 11:50:19 2021 +0000 Refactor to reduce JVM crashes on shutdown when sendfile is used --- java/org/apache/tomcat/util/net/AprEndpoint.java | 29 ++++++++++++++++++++---- webapps/docs/changelog.xml | 4 ++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java index 17e7fe4..c3d65a6 100644 --- a/java/org/apache/tomcat/util/net/AprEndpoint.java +++ b/java/org/apache/tomcat/util/net/AprEndpoint.java @@ -554,6 +554,10 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB // Stop the Poller calling select poller.stop(); + if (getUseSendfile()) { + sendfile.stop(); + } + // Wait for the acceptor to shutdown if (acceptor.getState() != AcceptorState.ENDED && !getBindOnInit()) { log.warn(sm.getString("endpoint.warn.unlockAcceptorFailed", acceptor.getThreadName())); @@ -578,8 +582,6 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB if (getUseSendfile()) { try { - sendfile.stop(); - // Wait for the sendfile thread to exit, otherwise parallel // destruction of sockets which are still in the poller can cause // problems. @@ -596,12 +598,9 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB if (sendfile.sendfileThread.isAlive()) { log.warn(sm.getString("endpoint.sendfileThreadStop")); } - - sendfile.destroy(); } catch (Exception e) { // Ignore } - sendfile = null; } // Close the SocketWrapper for each open connection - this should @@ -626,6 +625,15 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB Socket.shutdown(socket.longValue(), Socket.APR_SHUTDOWN_READWRITE); } + if (getUseSendfile()) { + try { + sendfile.destroy(); + } catch (Exception e) { + // Ignore + } + sendfile = null; + } + try { poller.destroy(); } catch (Exception e) { @@ -1806,6 +1814,15 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB * otherwise */ public SendfileState add(SendfileData data) { + + SocketWrapperBase<Long> socketWrapper = connections.get(Long.valueOf(data.socket)); + + // Use the blocking status write lock as a proxy for a lock on + // writing to the socket. Don't want it to close in another thread + // while this thread is writing as that could trigger a JVM crash. + WriteLock wl = ((AprSocketWrapper) socketWrapper).getBlockingStatusWriteLock(); + wl.lock(); + // Initialize fd from data given try { data.fdpool = Socket.pool(data.socket); @@ -1842,6 +1859,8 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB } catch (Exception e) { log.warn(sm.getString("endpoint.sendfile.error"), e); return SendfileState.ERROR; + } finally { + wl.unlock(); } // Add socket to the list. Newly added sockets will wait // at most for pollTime before being polled diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 6524485..5e5d69c 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -158,6 +158,10 @@ <bug>65677</bug>: Improve exception handling for errors during HTTP/1.1 reads with NIO2. (markt) </fix> + <fix> + Refactor APR/native connector shutdown to remove a potential source of + JVM crashes on shutdown when sendfile is used. (markt) + </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