You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@tomcat.apache.org by Manuel Dominguez Sarmiento <ma...@renxo.com> on 2020/02/18 14:20:56 UTC

OpenSSLEngine improvements to guard against multiple shutdown() calls triggered by construction exception and finalize() later

We started getting random SIGSEGV JVM crashes on our servers after 
upgrading to OpenJDK 13.0.2

The crashes are always in tcnative code, on the JVM Finalizer daemon 
thread, at OpenSSLEngine.shutdown(). We're on Tomcat 9.0.31, tcnative 
1.2.23, OpenSSL 1.1.1d and APR 1.7.0 using NIO connectors. This 
combination works fine with Oracle JDK 8u202 as well as OpenJDK 8u242, 
but crashes began immediately after upgrading to OpenJDK 13.0.2

Since the failure is related to tcnative code and finalization, we guess 
the new JVM is probably invoking the finalizers sooner, so this is 
exposing a bug in OpenSSLEngine which was already present, but was not 
being triggered on previous JVMs because of different timing.

The crash occurs when SSL.freeSSL() is invoked, so this probably means 
either the pointer is zero, or a double SSL.freeSSL() is invoked. This 
can happen if there are uncaught exceptions within the OpenSSLEngine 
constructor, as finalization can execute concurrently with object 
construction.

We adapted the solution found in OpenSSLContext.destroy() which 
evidently was causing similar issues. The current volatile "destroyed" 
checks found in OpenSSLEngine seem to be insufficient, so we replaced it 
with an AtomicBoolean. Also, we check whether "networkBIO" and "ssl" are 
zero in shutdown() to avoid attempting to free a null pointer.

We found a discussing regarding these issues in the following thread:
http://mail-archives.apache.org/mod_mbox/tomcat-dev/201901.mbox/%3C44ce40f1-36a1-df9b-b648-8635d9d84eb1@kippdata.de%3E

It seems the improvements made it to OpenSSLContext but not 
OpenSSLEngine. This pull request should fix remaining issues.

See attached sample HotSpot error dumps.

See the following GitHub pull request for the proposed changes:
https://github.com/apache/tomcat/pull/246

*Manuel Dominguez Sarmiento*