RFR [9] 8156841: sun.security.pkcs11.SunPKCS11 poller thread retains a strong reference to the context class loader

[Daniel Fuchs]

Hi Chris,

I agree that resetting the TCCL makes sense, and looks like the
minimal safer thing to do here.
InnocuousThread should probably be considered for the future,
but that's a first step in this direction :-)

best regards,

-- daniel

On 18/08/16 08:58, Chris Hegarty wrote:
>> The SunPKCS11 poller thread has no need of any user defined class loader,
>> so should set its context class loader to null before starting, so as to not
>> inadvertently retain a reference to the creating thread’s context class loader.
>>
>> In other areas that suffered from a similar issue we changed to use an
>> InnocuousThread, but I cannot fully satisfy myself that this is a safe
>> substation here, so I opted for the safest minimal fix. A future refactoring
>> exercise should consider using InnocuousThread.
>>
>> diff -r 92c31ec731eb src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/SunPKCS11.java
>> --- a/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/SunPKCS11.java	Wed Aug 10 11:54:12 2016 +0100
>> +++ b/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/SunPKCS11.java	Wed Aug 10 16:32:41 2016 +0100
>> @@ -809,20 +809,21 @@
>>         }
>>     }
>>
>>     // create the poller thread, if not already active
>>     private void createPoller() {
>>         if (poller != null) {
>>             return;
>>         }
>>         final TokenPoller poller = new TokenPoller(this);
>>         Thread t = new Thread(null, poller, "Poller " + getName(), 0, false);
>> +        t.setContextClassLoader(null);
>>         t.setDaemon(true);
>>         t.setPriority(Thread.MIN_PRIORITY);
>>         t.start();
>>         this.poller = poller;
>>     }
>>
>>     // destroy the poller thread, if active
>>     private void destroyPoller() {
>>         if (poller != null) {
>>             poller.disable();
>>
>> https://bugs.openjdk.java.net/browse/JDK-8156841
>>
>> -Chris.
>