Skip to content
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

Transaction support for signing and timed service - 2 #110

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ public class SignServerConstants {
*/
public static final String KEYUSAGELIMIT = "KEYUSAGELIMIT";
public static String DISABLEKEYUSAGECOUNTER = "DISABLEKEYUSAGECOUNTER";

public static String PROCESSINTRANSACTION = "PROCESSINTRANSACTION";

/**
* Constant used to set the default value of configuration property to NULL if not setting property means property value is NULL.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
*/
public class KeyAliasesCache extends CommonCacheBase<PublicKey> {

long lastUpdate = 0L;

@Override
public PublicKey getEntry(final Integer id) {
if (id == null) {
Expand All @@ -31,6 +33,10 @@ public PublicKey getEntry(final Integer id) {
return super.getEntry(id);
}

public void updateCacheTimeStamp() {
this.lastUpdate = System.currentTimeMillis();
}

@Override
protected long getCacheTime() {
return 30000; // Cache key aliases for 30 seconds
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,4 +209,8 @@ public WorkerType getWorkerType() {
}
return type;
}

public boolean requiresTransaction(final IServices services) {
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,11 @@ public interface IWorker {
* @return a WorkerStatus object.
*/
WorkerStatusInfo getStatus(final List<String> additionalFatalErrors, final IServices services);

/**
* If worker requires a database transaction when using this crypto token.
*
* @return True or false
*/
boolean requiresTransaction(final IServices services);
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,14 @@ public boolean isSingleton() {
return false;
}

/**
* @return Always false
*/
@Override
public boolean requiresTransaction(final IServices services) {
return false;
}

/**
* @return No log types
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,9 @@ public boolean isNoCertificatesRequired() {
return false;
}

@Override
public boolean requiresTransactionForSigning() {
return false;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -279,4 +279,11 @@ boolean removeKey(String alias, IServices services) throws CryptoTokenOfflineExc
* @return True or false
*/
boolean isNoCertificatesRequired();

/**
* If worker requires a database transaction for signing operation.
*
* @return True or false
*/
boolean requiresTransactionForSigning();
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
package org.signserver.server.signers;

import java.util.List;
import org.apache.log4j.Logger;
import org.signserver.common.WorkerStatusInfo;
import org.signserver.server.IServices;
import org.signserver.server.cryptotokens.ICryptoTokenV4;

/**
* Worker not performing any operations on its own.
Expand All @@ -25,6 +27,7 @@
*/
public class CryptoWorker extends NullSigner {

private static final Logger LOG = Logger.getLogger(CryptoWorker.class);
private static final String WORKER_TYPE = "CryptoWorker";

@Override
Expand All @@ -40,4 +43,17 @@ public WorkerStatusInfo getStatus(List<String> additionalFatalErrors, final ISer
return status;
}

public boolean requiresTransaction(final IServices services) {
try {
ICryptoTokenV4 cryptoToken = super.getCryptoToken(services);
if (cryptoToken == null) {
return false;
}
return cryptoToken.requiresTransactionForSigning();
} catch (Exception e) {
LOG.warn("Unable to determine whether a worker requires a transaction. Defaulting to False.", e);
return false;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ public boolean isSingleton() {
return active.trim().equalsIgnoreCase("TRUE");
}

@Override
public boolean requiresTransaction(final IServices services) {
return false;
}

@Override
public WorkerStatusInfo getStatus(final List<String> additionalFatalErrors, final IServices services) {
final List<String> fatalErrorsIncludingAdditionalErrors = new LinkedList<>(additionalFatalErrors);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import java.util.Set;
import org.signserver.common.ServiceContext;
import org.signserver.server.IServices;
import org.signserver.server.IWorker;
import org.signserver.server.ServiceExecutionFailedException;

Expand Down Expand Up @@ -62,6 +63,11 @@ public interface ITimedService extends IWorker {
* the time, of false if it should be run on all nodes simultaneously.
*/
boolean isSingleton();

/**
* @return true if the service requires a transaction to be executed successfully
*/
boolean requiresTransaction(final IServices services);

/**
* Get log types for logging work invocations.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public Response process(final AdminInfo adminInfo, final WorkerIdentifier wi,
throws IllegalRequestException, CryptoTokenOfflineException,
SignServerException {
requestContext.setServices(servicesImpl);
if (SessionUtils.needsTransaction(workerManagerSession, wi)) {
if (SessionUtils.needsTransaction(workerManagerSession, wi, servicesImpl)) {
// use separate transaction bean to avoid deadlock
return dispatcherProcessTransSession.processWithTransaction(adminInfo, wi, request, requestContext);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public Response process(final AdminInfo adminInfo, final WorkerIdentifier wi,
throws IllegalRequestException, CryptoTokenOfflineException,
SignServerException {
requestContext.setServices(servicesImpl);
if (SessionUtils.needsTransaction(workerManagerSession, wi)) {
if (SessionUtils.needsTransaction(workerManagerSession, wi, servicesImpl)) {
// use separate transaction bean to avoid deadlock
return internalProcessTransSession.processWithTransaction(adminInfo, wi, request, requestContext);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,8 +335,8 @@ public Response process(final AdminInfo adminInfo, final WorkerIdentifier wi,
if (LOG.isDebugEnabled()) {
LOG.debug(">process: " + wi);
}
if (SessionUtils.needsTransaction(workerManagerSession, wi)) {

if (SessionUtils.needsTransaction(workerManagerSession, wi, servicesImpl)) {
// use separate transaction bean to avoid deadlock
return processTransSession.processWithTransaction(adminInfo, wi, request, requestContext);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ public void ejbTimeout(Timer timer) {
ITimedService timedService = null;
boolean run = false;
boolean isSingleton = false;
boolean requiresTransaction = false;
UserTransaction ut = sessionCtx.getUserTransaction();
try {
ut.begin();
Expand All @@ -168,6 +169,7 @@ public void ejbTimeout(Timer timer) {
timedService = (ITimedService) worker;
sessionCtx.getTimerService().createSingleActionTimer(timedService.getNextInterval(), new TimerConfig(timerInfo, false));
isSingleton = timedService.isSingleton();
requiresTransaction = timedService.requiresTransaction(servicesImpl);
if (!isSingleton) {
run = true;
} else {
Expand Down Expand Up @@ -195,11 +197,16 @@ public void ejbTimeout(Timer timer) {
}
}

ut = sessionCtx.getUserTransaction();
if (run) {
if (serviceConfig != null && timedService != null) {
try {
if (timedService.isActive() && timedService.getNextInterval() != ITimedService.DONT_EXECUTE) {
timedService.work(new ServiceContext(servicesImpl));
if(requiresTransaction) {
ut.begin();
timedService.work(new ServiceContext(servicesImpl));
ut.commit();
}
serviceConfig.setLastRunTimestamp(new Date());
for (final ITimedService.LogType logType :
timedService.getLogTypes()) {
Expand All @@ -225,7 +232,8 @@ public void ejbTimeout(Timer timer) {
}

}
} catch (ServiceExecutionFailedException e) {
} catch (ServiceExecutionFailedException | SystemException | HeuristicRollbackException | NotSupportedException |
HeuristicMixedException | RollbackException e) {
// always log to error log, regardless of log types
// setup for service run logging
LOG.error("Service" + timerInfo + " execution failed. ", e);
Expand All @@ -240,6 +248,11 @@ public void ejbTimeout(Timer timer) {
timerInfo.toString(),
Collections.<String, Object>singletonMap("Message", e.getMessage()));
}
try {
ut.rollback();
} catch (SystemException ex) {
LOG.error("Rollback failed.", e);
}
} catch (RuntimeException e) {
/*
* DSS-377:
Expand All @@ -251,6 +264,11 @@ public void ejbTimeout(Timer timer) {
* since it is some kind of catastrophic failure..
*/
LOG.error("Service worker execution failed.", e);
try {
ut.rollback();
} catch (SystemException ex) {
throw new RuntimeException("Rollback failed", ex);
}
}
} else {
LOG.error("Service with ID " + timerInfo + " not found.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import org.signserver.ejb.worker.impl.PreloadedWorkerConfig;
import org.signserver.ejb.worker.impl.WorkerManagerSingletonBean;
import org.signserver.ejb.worker.impl.WorkerWithComponents;
import org.signserver.server.IServices;
import org.signserver.server.IWorker;

/**
* Utility functions for session beans.
Expand All @@ -33,13 +35,14 @@ public class SessionUtils {
* @return true if the request needs a transaction
*/
public static boolean needsTransaction(final WorkerManagerSingletonBean session,
final WorkerIdentifier wi) {
final WorkerIdentifier wi, IServices services) {
try {
final WorkerWithComponents worker = session.getWorkerWithComponents(wi);
final PreloadedWorkerConfig pwc = worker.getPreloadedConfig();
final WorkerWithComponents workerWithComponents = session.getWorkerWithComponents(wi);
final IWorker worker = workerWithComponents.getWorker();
final PreloadedWorkerConfig pwc = workerWithComponents.getPreloadedConfig();

return !worker.getArchivers().isEmpty() ||
!pwc.isDisableKeyUsageCounter() || pwc.isKeyUsageLimitSpecified();
return !workerWithComponents.getArchivers().isEmpty() ||
!pwc.isDisableKeyUsageCounter() || pwc.isKeyUsageLimitSpecified() || worker.requiresTransaction(services);
} catch (NoSuchWorkerException e) {
return false;
}
Expand Down