This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push: new c38a7ef Refactor to reduce JVM crashes on shutdown when sendfile is used c38a7ef is described below commit c38a7ef3125ec8ec221ec3f660050bc2800526bd 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 b25398b..676f0b2 100644 --- a/java/org/apache/tomcat/util/net/AprEndpoint.java +++ b/java/org/apache/tomcat/util/net/AprEndpoint.java @@ -494,6 +494,10 @@ public class AprEndpoint extends AbstractEndpoint<Long> implements SNICallBack { // Stop the Poller calling select poller.stop(); + if (getUseSendfile()) { + sendfile.stop(); + } + // Wait for the acceptor to shutdown. // Should only be one thread but retain this code in case the // acceptor start has been customised. @@ -534,8 +538,6 @@ public class AprEndpoint extends AbstractEndpoint<Long> implements SNICallBack { 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. @@ -552,12 +554,9 @@ public class AprEndpoint extends AbstractEndpoint<Long> implements SNICallBack { 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 @@ -584,6 +583,15 @@ public class AprEndpoint extends AbstractEndpoint<Long> implements SNICallBack { 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) { @@ -1875,6 +1883,15 @@ public class AprEndpoint extends AbstractEndpoint<Long> implements SNICallBack { * 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); @@ -1911,6 +1928,8 @@ public class AprEndpoint extends AbstractEndpoint<Long> implements SNICallBack { } 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 2c0685f..9a9af08 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -151,6 +151,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