-
Notifications
You must be signed in to change notification settings - Fork 18
Implement cleaner to replace deprecated finalizers #845
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,10 @@ | |
package com.ibm.crypto.plus.provider; | ||
|
||
import com.ibm.crypto.plus.provider.ock.OCKContext; | ||
import java.lang.ref.Cleaner; | ||
import java.security.ProviderException; | ||
import java.util.concurrent.atomic.AtomicInteger; | ||
import sun.security.util.Debug; | ||
|
||
// Internal interface for OpenJCEPlus and OpenJCEPlus implementation classes. | ||
// Implemented as an abstract class rather than an interface so that | ||
|
@@ -31,8 +34,29 @@ public abstract class OpenJCEPlusProvider extends java.security.Provider { | |
// private static boolean verifiedSelfIntegrity = false; | ||
private static final boolean verifiedSelfIntegrity = true; | ||
|
||
private final Cleaner[] cleaners; | ||
|
||
private final int DEFAULT_NUM_CLEANERS = 2; | ||
|
||
private final int numCleaners; | ||
|
||
private AtomicInteger count = new AtomicInteger(0); | ||
|
||
private static final boolean isDebugSet = Debug.getInstance(DEBUG_VALUE) != null ? true : false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both the
Seems like we could eliminate this in those provider classes and use the new |
||
|
||
OpenJCEPlusProvider(String name, String info) { | ||
super(name, PROVIDER_VER, info); | ||
|
||
numCleaners = Integer.getInteger("openjceplus.cleaners.num", DEFAULT_NUM_CLEANERS); | ||
if (numCleaners < 1){ | ||
throw new IllegalArgumentException(numCleaners + " is an invalid number of cleaner threads, must be at least 1."); | ||
} | ||
|
||
cleaners = new Cleaner[numCleaners]; | ||
for (int i = 0; i < numCleaners; i++) { | ||
final Cleaner cleaner = Cleaner.create(); | ||
cleaners[i] = cleaner; | ||
} | ||
} | ||
|
||
static final boolean verifySelfIntegrity(Object c) { | ||
|
@@ -47,6 +71,15 @@ private static final synchronized boolean doSelfVerification(Object c) { | |
return true; | ||
} | ||
|
||
public void registerCleanable(Object owner, Runnable cleanAction) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this method need to be public? Not sure we want users making calls to this method. |
||
Cleaner cleaner = cleaners[Math.abs(count.getAndIncrement() % numCleaners)]; | ||
cleaner.register(owner, cleanAction); | ||
} | ||
|
||
public boolean getDebug() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this method need to be public? Not sure we want users making calls to this method. |
||
return isDebugSet; | ||
} | ||
|
||
// Get OCK context for crypto operations | ||
// | ||
abstract OCKContext getOCKContext(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
|
||
package com.ibm.crypto.plus.provider.ock; | ||
|
||
import com.ibm.crypto.plus.provider.OpenJCEPlusProvider; | ||
import java.util.concurrent.ConcurrentLinkedQueue; | ||
import java.util.stream.Collectors; | ||
import java.util.stream.Stream; | ||
|
@@ -26,7 +27,7 @@ public final class Digest implements Cloneable { | |
// -2 : Not a SHA* digest algorithm | ||
private int algIndx = -1; | ||
|
||
private boolean needsReinit = false; | ||
private BoolWrapper needsReinit = new BoolWrapper(false); | ||
|
||
private boolean contextFromQueue = false; | ||
|
||
|
@@ -130,44 +131,28 @@ void getContext() throws OCKException { | |
this.contextFromQueue = true; | ||
} | ||
} | ||
this.needsReinit = false; | ||
this.needsReinit.setValue(false); | ||
} | ||
|
||
void releaseContext() throws OCKException { | ||
/* end digest caching mechanism | ||
* =========================================================================== | ||
*/ | ||
|
||
if (this.digestId == 0) { | ||
return; | ||
/* This wrapper is used to pass a primitive variable as a parameter by reference instead of by value to the cleaner. */ | ||
public class BoolWrapper { | ||
boolean value; | ||
public BoolWrapper(boolean value) { | ||
this.value = value; | ||
} | ||
|
||
// not SHA* algorithm | ||
if (this.algIndx == -2) { | ||
if (validId(this.digestId)) { | ||
NativeInterface.DIGEST_delete(this.ockContext.getId(), | ||
this.digestId); | ||
this.digestId = 0; | ||
} | ||
} else { | ||
if (this.contextFromQueue) { | ||
// reset now to make sure all contexts in the queue are ready to use | ||
this.reset(); | ||
contexts[this.algIndx].add(this.digestId); | ||
this.digestId = 0; | ||
this.contextFromQueue = false; | ||
} else { | ||
// delete context | ||
if (validId(this.digestId)) { | ||
NativeInterface.DIGEST_delete(this.ockContext.getId(), | ||
this.digestId); | ||
this.digestId = 0; | ||
} | ||
} | ||
public boolean getValue(){ | ||
return this.value; | ||
} | ||
this.digestId = 0; | ||
} | ||
|
||
/* end digest caching mechanism | ||
* =========================================================================== | ||
*/ | ||
public void setValue(boolean value) { | ||
this.value = value; | ||
} | ||
} | ||
|
||
private OCKContext ockContext = null; | ||
private int digestLength = 0; | ||
|
@@ -178,7 +163,9 @@ void releaseContext() throws OCKException { | |
|
||
private long digestId = 0; | ||
|
||
public static Digest getInstance(OCKContext ockContext, String digestAlgo) throws OCKException { | ||
private OpenJCEPlusProvider provider; | ||
|
||
public static Digest getInstance(OCKContext ockContext, String digestAlgo, OpenJCEPlusProvider provider) throws OCKException { | ||
KostasTsiounis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (ockContext == null) { | ||
throw new IllegalArgumentException("context is null"); | ||
} | ||
|
@@ -187,15 +174,23 @@ public static Digest getInstance(OCKContext ockContext, String digestAlgo) throw | |
throw new IllegalArgumentException("digestAlgo is null/empty"); | ||
} | ||
|
||
return new Digest(ockContext, digestAlgo); | ||
return new Digest(ockContext, digestAlgo, provider); | ||
} | ||
|
||
private Digest(OCKContext ockContext, String digestAlgo) throws OCKException { | ||
private Digest(OCKContext ockContext, String digestAlgo, OpenJCEPlusProvider provider) throws OCKException { | ||
//final String methodName = "Digest(String)"; | ||
this.ockContext = ockContext; | ||
this.digestAlgo = digestAlgo; | ||
this.provider = provider; | ||
getContext(); | ||
//OCKDebug.Msg(debPrefix, methodName, "digestAlgo :" + digestAlgo); | ||
|
||
if (provider == null) { | ||
throw new IllegalArgumentException("Provider cannot be null."); | ||
} | ||
|
||
this.provider.registerCleanable(this, cleanOCKResources(digestId, algIndx, | ||
contextFromQueue, needsReinit, ockContext, this.provider.getDebug())); | ||
} | ||
|
||
private Digest() { | ||
|
@@ -238,7 +233,7 @@ public synchronized void update(byte[] input, int offset, int length) throws OCK | |
if (errorCode < 0) { | ||
throwOCKException(errorCode); | ||
} | ||
this.needsReinit = true; | ||
this.needsReinit.setValue(true); | ||
} | ||
|
||
public synchronized byte[] digest() throws OCKException { | ||
|
@@ -260,7 +255,7 @@ public synchronized byte[] digest() throws OCKException { | |
if (errorCode < 0) { | ||
throwOCKException(errorCode); | ||
} | ||
this.needsReinit = false; | ||
this.needsReinit.setValue(false); | ||
|
||
return digestBytes; | ||
} | ||
|
@@ -292,10 +287,10 @@ public synchronized void reset() throws OCKException { | |
if (!validId(this.digestId)) { | ||
throw new OCKException(badIdMsg); | ||
} | ||
if (this.needsReinit) { | ||
if (this.needsReinit.getValue() == true) { | ||
NativeInterface.DIGEST_reset(this.ockContext.getId(), this.digestId); | ||
} | ||
this.needsReinit = false; | ||
this.needsReinit.setValue(false);; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are two |
||
} | ||
|
||
private synchronized void obtainDigestLength() throws OCKException { | ||
|
@@ -317,18 +312,6 @@ private synchronized void obtainDigestLength() throws OCKException { | |
} | ||
} | ||
|
||
@Override | ||
protected synchronized void finalize() throws Throwable { | ||
//final String methodName = "finalize"; | ||
|
||
try { | ||
//OCKDebug.Msg(debPrefix, methodName, "digestId =" + this.digestId); | ||
releaseContext(); | ||
jasonkatonica marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} finally { | ||
super.finalize(); | ||
} | ||
} | ||
|
||
/* At some point we may enhance this function to do other validations */ | ||
protected static boolean validId(long id) { | ||
//final String methodName = "validId"; | ||
|
@@ -348,9 +331,10 @@ public synchronized Object clone() throws CloneNotSupportedException { | |
copy.digestLength = this.digestLength; | ||
copy.algIndx = this.algIndx; | ||
copy.digestAlgo = new String(this.digestAlgo); | ||
copy.needsReinit = this.needsReinit; | ||
copy.needsReinit.setValue(this.needsReinit.getValue()); | ||
copy.ockContext = this.ockContext; | ||
copy.contextFromQueue = false; | ||
copy.provider = this.provider; | ||
|
||
// Allocate a new context for the digestId and copy all state information from our | ||
// original context into the copy. | ||
|
@@ -367,6 +351,41 @@ public synchronized Object clone() throws CloneNotSupportedException { | |
.collect(Collectors.joining("\n")); | ||
throw new CloneNotSupportedException(stackTrace); | ||
} | ||
|
||
this.provider.registerCleanable(copy, cleanOCKResources(copy.digestId, copy.algIndx, | ||
copy.contextFromQueue, copy.needsReinit, copy.ockContext, this.provider.getDebug())); | ||
return copy; | ||
} | ||
|
||
private Runnable cleanOCKResources(long digestId, int algIndx, boolean contextFromQueue, | ||
BoolWrapper needsReinit, OCKContext ockContext, boolean debug) { | ||
return () -> { | ||
try { | ||
if (digestId == 0) { | ||
throw new OCKException("Digest Identifier is not valid"); | ||
} | ||
// not SHA* algorithm | ||
if (algIndx == -2) { | ||
if (validId(digestId)) { | ||
NativeInterface.DIGEST_delete(ockContext.getId(), digestId); | ||
} | ||
} else { | ||
if (contextFromQueue) { | ||
// reset now to make sure all contexts in the queue are ready to use | ||
if (needsReinit.getValue()) { | ||
NativeInterface.DIGEST_reset(ockContext.getId(), digestId); | ||
} | ||
Digest.contexts[algIndx].add(digestId); | ||
} else { | ||
NativeInterface.DIGEST_delete(ockContext.getId(), digestId); | ||
} | ||
} | ||
} catch (OCKException e) { | ||
if (debug) { | ||
System.out.println("An error occurred while cleaning : " + e.getMessage()); | ||
e.printStackTrace(); | ||
} | ||
} | ||
}; | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.