[matrei]

BouncyCastle's CryptoServicesRegistrar stores its default DSA and DH parameters globally, and also in a thread-local of the thread that initializes it, see bcgit/bc-java#2536. That thread keeps them, and through their classes BouncyCastle's class loader, for as long as it lives. When JRuby is embedded in a class loader that's thrown away later, as in jruby/jruby#9781, JRuby's class loader stays alive with it.

This change clears the thread's copies in OpenSSL.doLoad. Lookups fall back to the global values, which hold the same parameters, so nothing else changes. Calling clearThreadProperty also initializes the registrar on the loading thread, so that's always the thread whose copies get cleared.

Class loaders still alive after four runtimes did require 'openssl', a digest, AES, an RSA signature and SSLContext#setup, with the reproducer from jruby/jruby#9950 and -XX:SoftRefLRUPolicyMSPerMB=0, on JDK 21 and 25:

jruby-openssl	as is	with this change
0.16.3, in JRuby 10.0's jar	4 of 4	0 of 4
master, loaded from lib	4 of 4	1 of 4

On master, the last one is held by the SecureRandom that Random.ThreadLocalHolder keeps per thread since c08aa18, whose provider is BouncyCastle's. With -Djruby.openssl.random=shared, it's 0 of 4. I'd look at that one separately.

With JRuby 10.0, ./mvnw test, rake test, rake test:provider:bc_all and rake test:provider:bc_wout_jsse give the same results as on master: 135 JUnit tests pass, and each rake target runs 892 tests with 0 failures and 0 errors.

JRuby 10.0 bundles 0.16.3. The same change for the 0.16 branch is in matrei:fix/clear-bc-thread-properties-0.16, if you'd like it there too.

bc-fips has the same API, so the call works in the FIPS variant too. But bc-fips also registers a shutdown hook that keeps its class loader alive regardless.

[kares]

Isn't this a bit futile, there's no guarantee it will be clear by the time runtime is being cleared down?

There's a lot of explanation, but the interesting part would be the use-case and the actual setup, when this happens.
I wonder if there's more thread-locals in BC that would initialize upon usage and prevent tear-down anyway.

I'll follow the upstream response, not sure we should be messing with the explicit registrar configuration...

[matrei]

The setup is the one in jruby/jruby#9781: a host runs Ruby code in a JRuby runtime in its own class loader, tears it down and drops the loader, while its threads live on. The Gradle daemon does this in asciidoctor/asciidoctor-gradle-plugin#810. Anything there that loads openssl, like Net::HTTP with https, then keeps the loader alive.

Apart from the registrar's static initializer, only setThreadProperty and setGlobalProperty set these thread properties, and neither BC nor jruby-openssl calls them. So once they're cleared on load, they stay clear. The numbers in the description are counted after teardown.

BC 1.86 has seven other classes with thread-locals. I checked them: each one holds only JDK objects, is removed in a finally, or is only set by explicit configuration or by jsse.util classes that jruby-openssl doesn't use. That includes the registrar's per-thread default SecureRandom, which BC fills on use. With it filled, this change still gives 0 of 4 on 0.16.

Following upstream sounds good to me. A fix there would also cover a case this change can't tell apart: if the application set a DSA or DH thread property on the loading thread itself, this would clear that too.

[headius]

@kares This is part of ongoing exploration to eliminate as many lingering roots as possible. This started out as jruby/jruby#9781 but once the original use case was solved there I requested that @matrei move further exploration to the discussion at jruby/jruby#9950. This is one of the additional cases discovered along the way.

I know this is a very long tail, since many (most?) Java libraries abuse thread-locals and static caches and we can't fix them all. It's just a best effort to try to find as many fixable scenarios as possible, to avoid future reports of classloader leaks after runtime teardown.

[kares]

Thanks, I've been following those - just wasn't sure if it's driven by actual production use-cases.
If there's no actual blocker here I would treat this as a lower priority, watching over for feedback from BC folks.

We might still need to do a detailed audit of thread-locals within BC, some of the JDK stuff could be extended (by BC).
That is going to still mean they might leak unless you move the library away from the JRuby class-loader and put BC jars on the parent (system) loader.

An actual interesting use-case would be having JOSSL do all kinds of operations, hitting BC thread-locals, and checking if anything else leaks.