On Mon, Feb 12, 2024 at 4:05 PM Christopher Schultz <ch...@christopherschultz.net> wrote: > > Mark, > > On 2/8/24 16:10, ma...@apache.org wrote: > > This is an automated email from the ASF dual-hosted git repository. > > > > markt pushed a commit to branch main > > in repository https://gitbox.apache.org/repos/asf/tomcat.git > > > > commit 543e2b56bc8ccbde973366975b211b61408caf8a > > Author: Mark Thomas <ma...@apache.org> > > AuthorDate: Thu Feb 8 18:54:45 2024 +0000 > > > > Reduce changes of crash on Library shutdown with OpenSSL connections > > --- > > .../tomcat/util/net/openssl/OpenSSLContext.java | 19 > > +++++++++++-------- > > 1 file changed, 11 insertions(+), 8 deletions(-) > > > > diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java > > b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java > > index f1d7b092ec..12dc41455b 100644 > > --- a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java > > +++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java > > @@ -46,6 +46,7 @@ import javax.net.ssl.X509TrustManager; > > import org.apache.juli.logging.Log; > > import org.apache.juli.logging.LogFactory; > > import org.apache.tomcat.jni.CertificateVerifier; > > +import org.apache.tomcat.jni.Library; > > import org.apache.tomcat.jni.Pool; > > import org.apache.tomcat.jni.SSL; > > import org.apache.tomcat.jni.SSLConf; > > @@ -648,14 +649,16 @@ public class OpenSSLContext implements > > org.apache.tomcat.util.net.SSLContext { > > > > @Override > > public void run() { > > - if (ctx != 0) { > > - SSLContext.free(ctx); > > - } > > - if (cctx != 0) { > > - SSLConf.free(cctx); > > - } > > - if (aprPool != 0) { > > - Pool.destroy(aprPool); > > + if (Library.isInitialized()) { > > + if (ctx != 0) { > > + SSLContext.free(ctx); > > + } > > + if (cctx != 0) { > > + SSLConf.free(cctx); > > + } > > + if (aprPool != 0) { > > + Pool.destroy(aprPool); > > + } > > Should this be in a synchronized block? Probably using Library as the > monitor? > > Also, maybe zero everything out afterward? > > Honestly, this looks weird to my (untrained) eye. You are checking the > state of Library to determine whether or not local resources ought to be > cleaned up? That smells funny to me...
The crashes in CI were caused by https://github.com/apache/tomcat/commit/6ce18dc93a054949e529952e809b159040b1d158 and weird interactions with the APR connector. The fix for the CI crashes is: https://github.com/apache/tomcat/commit/4cc023ff17f874ac653d6731106dd4b062686d15 Beyond that, if the plan is to make all of this really much safer, then FFM is really a big step in that direction. Rémy > -chris > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
