Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fix memory leak on each TLS connection
Each TLS connection is leaking a bunch of data that isn't in the heap
and so after 25k requests Tomcat uses about 2.5GB resident memory.

There are large number of relationships that point at each other and we
need to break the cycle so JSSEngineReferenceImpl's finalizer can run
and clear all the native resources these point at.

The lowest impact place to break the cycle was at SSLAlertEvent.engine.
This relationship doesn't seem to be used anywhere.  Once the cycle is
broken, JSSEngineReferenceImpl can be garbage collected and the
finalizer can run.

Signed-off-by: Chris Kelley <ckelley@redhat.com>
  • Loading branch information
uplogix-mmcclain authored and ckelleyRH committed Feb 11, 2022
1 parent 0eb6879 commit 5922560
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 21 deletions.
16 changes: 8 additions & 8 deletions src/main/java/org/mozilla/jss/nss/SSLFDProxy.java
@@ -1,6 +1,5 @@
package org.mozilla.jss.nss;

import java.lang.IllegalArgumentException;
import java.util.ArrayList;

import org.mozilla.jss.crypto.X509Certificate;
Expand Down Expand Up @@ -42,13 +41,14 @@ public void SetClientCert(X509Certificate cert) throws IllegalArgumentException

@Override
protected synchronized void releaseNativeResources() throws Exception {
synchronized (globalRef) {
if (globalRef != null) {
try {
globalRef.close();
} finally {
globalRef = null;
}

super.releaseNativeResources();

if (globalRef != null) {
try {
globalRef.close();
} finally {
globalRef = null;
}
}
}
Expand Down
51 changes: 41 additions & 10 deletions src/main/java/org/mozilla/jss/ssl/javax/JSSEngineReferenceImpl.java
Expand Up @@ -4,17 +4,39 @@
import java.io.OutputStream;
import java.net.ServerSocket;
import java.net.Socket;
import java.nio.channels.WritableByteChannel;
import java.nio.ByteBuffer;
import java.nio.channels.Channels;
import java.nio.channels.WritableByteChannel;
import java.security.PublicKey;
import java.nio.ByteBuffer;

import javax.net.ssl.*;

import org.mozilla.jss.nss.*;
import org.mozilla.jss.pkcs11.*;
import org.mozilla.jss.provider.javax.crypto.*;
import org.mozilla.jss.ssl.*;
import javax.net.ssl.SSLEngineResult;
import javax.net.ssl.SSLException;
import javax.net.ssl.SSLHandshakeException;
import javax.net.ssl.X509ExtendedTrustManager;
import javax.net.ssl.X509TrustManager;

import org.mozilla.jss.nss.BadCertHandler;
import org.mozilla.jss.nss.Buffer;
import org.mozilla.jss.nss.BufferProxy;
import org.mozilla.jss.nss.Cert;
import org.mozilla.jss.nss.CertAuthHandler;
import org.mozilla.jss.nss.PR;
import org.mozilla.jss.nss.PRErrors;
import org.mozilla.jss.nss.PRFDProxy;
import org.mozilla.jss.nss.SSL;
import org.mozilla.jss.nss.SSLErrors;
import org.mozilla.jss.nss.SSLFDProxy;
import org.mozilla.jss.nss.SSLPreliminaryChannelInfo;
import org.mozilla.jss.nss.SecurityStatusResult;
import org.mozilla.jss.pkcs11.PK11Cert;
import org.mozilla.jss.provider.javax.crypto.JSSNativeTrustManager;
import org.mozilla.jss.ssl.SSLAlertDescription;
import org.mozilla.jss.ssl.SSLAlertEvent;
import org.mozilla.jss.ssl.SSLAlertLevel;
import org.mozilla.jss.ssl.SSLCipher;
import org.mozilla.jss.ssl.SSLHandshakeCompletedEvent;
import org.mozilla.jss.ssl.SSLVersion;
import org.mozilla.jss.ssl.SSLVersionRange;

/**
* The reference JSSEngine implementation.
Expand Down Expand Up @@ -1651,10 +1673,10 @@ private void cleanupSSLFD() {
if (!closed_fd && ssl_fd != null) {
try {
SSL.RemoveCallbacks(ssl_fd);
PR.Close(ssl_fd);
ssl_fd.close();
ssl_fd = null;
} catch (Exception e) {
debug("Got exception trying to cleanup SSLFD: " + e.getMessage());
logger.error("Got exception trying to cleanup SSLFD", e);
} finally {
closed_fd = true;
}
Expand All @@ -1671,6 +1693,15 @@ private void cleanupSSLFD() {
}
}

// During testing with Tomcat 8.5, most instances did not call
// cleanup, so all the JNI resources end up getting leaked: ssl_fd
// (and its global ref), read_buf, and write_buf.
@Override
protected void finalize() {
cleanup();
}


private class CertValidationTask extends CertAuthHandler {
public CertValidationTask(SSLFDProxy fd) {
super(fd);
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/org/mozilla/jss/util/GlobalRefProxy.java
Expand Up @@ -12,7 +12,5 @@ public GlobalRefProxy(Object target) {
private static native byte[] refOf(Object target);

@Override
protected synchronized void releaseNativeResources() {
clear();
}
protected native void releaseNativeResources();
}

0 comments on commit 5922560

Please sign in to comment.