From 8fc06d9c794c704eed2ab8707768fc72cc677ded Mon Sep 17 00:00:00 2001 From: Andrew Branson Date: Mon, 12 May 2014 16:55:54 +0200 Subject: Tweak locks to hopefully avoid deadlocks a bit --- .../c2kernel/persistency/TransactionManager.java | 89 +++++++++++++--------- 1 file changed, 52 insertions(+), 37 deletions(-) (limited to 'src/main/java/com/c2kernel/persistency/TransactionManager.java') diff --git a/src/main/java/com/c2kernel/persistency/TransactionManager.java b/src/main/java/com/c2kernel/persistency/TransactionManager.java index 86e8199..d966eec 100644 --- a/src/main/java/com/c2kernel/persistency/TransactionManager.java +++ b/src/main/java/com/c2kernel/persistency/TransactionManager.java @@ -63,16 +63,14 @@ public class TransactionManager { Integer sysKeyIntObj = new Integer(sysKey); // check to see if the locker has been modifying this cluster - synchronized(locks) { - if (locks.containsKey(sysKeyIntObj) && locks.get(sysKeyIntObj).equals(locker)) { - ArrayList lockerTransaction = pendingTransactions.get(locker); - for (TransactionEntry thisEntry : lockerTransaction) { - if (sysKey == thisEntry.sysKey.intValue() && path.equals(thisEntry.getPath())) { - if (thisEntry.obj == null) - throw new ClusterStorageException("ClusterStorageManager.get() - Cluster " + path + " has been deleted in " + sysKey + - " but not yet committed"); - return thisEntry.obj; - } + if (locks.containsKey(sysKeyIntObj) && locks.get(sysKeyIntObj).equals(locker)) { + ArrayList lockerTransaction = pendingTransactions.get(locker); + for (TransactionEntry thisEntry : lockerTransaction) { + if (sysKey == thisEntry.sysKey.intValue() && path.equals(thisEntry.getPath())) { + if (thisEntry.obj == null) + throw new ClusterStorageException("ClusterStorageManager.get() - Cluster " + path + " has been deleted in " + sysKey + + " but not yet committed"); + return thisEntry.obj; } } } @@ -85,6 +83,7 @@ public class TransactionManager { */ public void put(int sysKey, C2KLocalObject obj, Object locker) throws ClusterStorageException { Integer sysKeyIntObj = new Integer(sysKey); + Object tempLocker = null; ArrayList lockerTransaction; String path = ClusterStorage.getPath(obj); @@ -99,28 +98,35 @@ public class TransactionManager { throw new ClusterStorageException("ClusterStorageManager.get() - Access denied: Object " + sysKeyIntObj + " has been locked for writing by " + thisLocker); } - else { // either we are the locker, or there is no locker - if (locker == null) { // non-locking put/delete - storage.put(sysKeyIntObj, obj); - return; + else { // no locks for this item + if (locker == null) { // lock the item until the non-transactional put is complete :/ + tempLocker = new Object(); + locks.put(sysKeyIntObj, tempLocker); + lockerTransaction = null; } - else {// initialise the transaction + else { // initialise the transaction locks.put(sysKeyIntObj, locker); lockerTransaction = new ArrayList(); pendingTransactions.put(locker, lockerTransaction); } } - - // create the new entry in the transaction table - TransactionEntry newEntry = new TransactionEntry(sysKeyIntObj, path, obj); - /* equals() in TransactionEntry only compares sysKey and path, so we can use - * contains() in ArrayList to looks for preexisting entries for this cluster - * and overwrite them. - */ - if (lockerTransaction.contains(newEntry)) - lockerTransaction.remove(newEntry); - lockerTransaction.add(newEntry); } + + if (tempLocker != null) { // non-locking put/delete + storage.put(sysKeyIntObj, obj); + locks.remove(sysKeyIntObj); + return; + } + + // create the new entry in the transaction table + TransactionEntry newEntry = new TransactionEntry(sysKeyIntObj, path, obj); + /* equals() in TransactionEntry only compares sysKey and path, so we can use + * contains() in ArrayList to looks for preexisting entries for this cluster + * and overwrite them. + */ + if (lockerTransaction.contains(newEntry)) + lockerTransaction.remove(newEntry); + lockerTransaction.add(newEntry); } /** Public delete method. Uses the put method, with null as the object value. @@ -128,6 +134,7 @@ public class TransactionManager { public void remove(int sysKey, String path, Object locker) throws ClusterStorageException { Integer sysKeyIntObj = new Integer(sysKey); ArrayList lockerTransaction; + Object tempLocker = null; synchronized(locks) { // look to see if this object is already locked if (locks.containsKey(sysKeyIntObj)) { @@ -141,8 +148,9 @@ public class TransactionManager { } else { // either we are the locker, or there is no locker if (locker == null) { // non-locking put/delete - storage.remove(sysKeyIntObj, path); - return; + tempLocker = new Object(); + locks.put(sysKeyIntObj, tempLocker); + lockerTransaction = null; } else {// initialise the transaction locks.put(sysKeyIntObj, locker); @@ -150,17 +158,24 @@ public class TransactionManager { pendingTransactions.put(locker, lockerTransaction); } } - - // create the new entry in the transaction table - TransactionEntry newEntry = new TransactionEntry(sysKeyIntObj, path, null); - /* equals() in TransactionEntry only compares sysKey and path, so we can use - * contains() in ArrayList to looks for preexisting entries for this cluster - * and overwrite them. - */ - if (lockerTransaction.contains(newEntry)) - lockerTransaction.remove(newEntry); - lockerTransaction.add(newEntry); } + + if (tempLocker != null) { + storage.remove(sysKeyIntObj, path); + locks.remove(sysKeyIntObj); + return; + } + + // create the new entry in the transaction table + TransactionEntry newEntry = new TransactionEntry(sysKeyIntObj, path, null); + /* equals() in TransactionEntry only compares sysKey and path, so we can use + * contains() in ArrayList to looks for preexisting entries for this cluster + * and overwrite them. + */ + if (lockerTransaction.contains(newEntry)) + lockerTransaction.remove(newEntry); + lockerTransaction.add(newEntry); + } /** -- cgit v1.2.3 From 49fe139f52afcb444478400d10db41263ef1162d Mon Sep 17 00:00:00 2001 From: Andrew Branson Date: Thu, 5 Jun 2014 15:28:46 +0200 Subject: Add Authenticator to the open() method params of ClusterStorage. Passed in through the TransactionManager. This allows user-login to storages. Fixes #192 --- src/main/java/com/c2kernel/persistency/ClusterStorage.java | 3 ++- src/main/java/com/c2kernel/persistency/ClusterStorageManager.java | 5 +++-- src/main/java/com/c2kernel/persistency/LDAPClusterStorage.java | 3 ++- src/main/java/com/c2kernel/persistency/MemoryOnlyClusterStorage.java | 3 ++- src/main/java/com/c2kernel/persistency/ProxyLoader.java | 3 ++- src/main/java/com/c2kernel/persistency/TransactionManager.java | 5 +++-- src/main/java/com/c2kernel/persistency/XMLClusterStorage.java | 5 +++-- src/main/java/com/c2kernel/process/Gateway.java | 4 ++-- 8 files changed, 19 insertions(+), 12 deletions(-) (limited to 'src/main/java/com/c2kernel/persistency/TransactionManager.java') diff --git a/src/main/java/com/c2kernel/persistency/ClusterStorage.java b/src/main/java/com/c2kernel/persistency/ClusterStorage.java index 9c18bb4..76aaf1e 100644 --- a/src/main/java/com/c2kernel/persistency/ClusterStorage.java +++ b/src/main/java/com/c2kernel/persistency/ClusterStorage.java @@ -3,6 +3,7 @@ package com.c2kernel.persistency; import com.c2kernel.entity.C2KLocalObject; import com.c2kernel.persistency.outcome.Outcome; import com.c2kernel.persistency.outcome.Viewpoint; +import com.c2kernel.process.auth.Authenticator; import com.c2kernel.utils.Logger; /** Interface for persistency managers of entities. It allows different kernel objects to be stored in different backend. For instance, @@ -85,7 +86,7 @@ public abstract class ClusterStorage { public static final String[] allClusterTypes = { PROPERTY, COLLECTION, LIFECYCLE, OUTCOME, HISTORY, VIEWPOINT, JOB }; // connection maintenance - public abstract void open() + public abstract void open(Authenticator auth) throws ClusterStorageException; public abstract void close() throws ClusterStorageException; diff --git a/src/main/java/com/c2kernel/persistency/ClusterStorageManager.java b/src/main/java/com/c2kernel/persistency/ClusterStorageManager.java index 20857c6..c9ede04 100644 --- a/src/main/java/com/c2kernel/persistency/ClusterStorageManager.java +++ b/src/main/java/com/c2kernel/persistency/ClusterStorageManager.java @@ -15,6 +15,7 @@ import com.c2kernel.events.History; import com.c2kernel.persistency.outcome.Outcome; import com.c2kernel.persistency.outcome.Viewpoint; import com.c2kernel.process.Gateway; +import com.c2kernel.process.auth.Authenticator; import com.c2kernel.utils.Logger; import com.c2kernel.utils.SoftCache; import com.c2kernel.utils.WeakCache; @@ -38,7 +39,7 @@ public class ClusterStorageManager { * Initialises all ClusterStorage handlers listed by class name in the property "ClusterStorages" * This property is usually process specific, and so should be in the server/client.conf and not the connect file. */ - public ClusterStorageManager() throws ClusterStorageException { + public ClusterStorageManager(Authenticator auth) throws ClusterStorageException { Object clusterStorageProp = Gateway.getProperties().getObject("ClusterStorage"); if (clusterStorageProp == null || clusterStorageProp.equals("")) { throw new ClusterStorageException("ClusterStorageManager.init() - no ClusterStorages defined. No persistency!"); @@ -64,7 +65,7 @@ public class ClusterStorageManager { int clusterNo = 0; for (ClusterStorage newStorage : rootStores) { try { - newStorage.open(); + newStorage.open(auth); } catch (ClusterStorageException ex) { Logger.error(ex); throw new ClusterStorageException("ClusterStorageManager.init() - Error initialising storage handler " + newStorage.getClass().getName() + diff --git a/src/main/java/com/c2kernel/persistency/LDAPClusterStorage.java b/src/main/java/com/c2kernel/persistency/LDAPClusterStorage.java index cc65805..4762a33 100644 --- a/src/main/java/com/c2kernel/persistency/LDAPClusterStorage.java +++ b/src/main/java/com/c2kernel/persistency/LDAPClusterStorage.java @@ -10,6 +10,7 @@ import com.c2kernel.lookup.Lookup; import com.c2kernel.lookup.ldap.LDAPLookup; import com.c2kernel.lookup.ldap.LDAPPropertyManager; import com.c2kernel.process.Gateway; +import com.c2kernel.process.auth.Authenticator; import com.c2kernel.property.Property; import com.c2kernel.utils.Logger; @@ -17,7 +18,7 @@ public class LDAPClusterStorage extends ClusterStorage { LDAPPropertyManager ldapStore; @Override - public void open() throws ClusterStorageException { + public void open(Authenticator auth) throws ClusterStorageException { Lookup lookup = Gateway.getLookup(); if (lookup instanceof LDAPLookup) ldapStore = ((LDAPLookup)lookup).getPropManager(); diff --git a/src/main/java/com/c2kernel/persistency/MemoryOnlyClusterStorage.java b/src/main/java/com/c2kernel/persistency/MemoryOnlyClusterStorage.java index 4766d82..cd5d122 100644 --- a/src/main/java/com/c2kernel/persistency/MemoryOnlyClusterStorage.java +++ b/src/main/java/com/c2kernel/persistency/MemoryOnlyClusterStorage.java @@ -6,6 +6,7 @@ import java.util.HashMap; import java.util.Map; import com.c2kernel.entity.C2KLocalObject; +import com.c2kernel.process.auth.Authenticator; import com.c2kernel.utils.Logger; public class MemoryOnlyClusterStorage extends ClusterStorage { @@ -19,7 +20,7 @@ public class MemoryOnlyClusterStorage extends ClusterStorage { } @Override - public void open() throws ClusterStorageException { + public void open(Authenticator auth) throws ClusterStorageException { } diff --git a/src/main/java/com/c2kernel/persistency/ProxyLoader.java b/src/main/java/com/c2kernel/persistency/ProxyLoader.java index fe48966..57b91af 100644 --- a/src/main/java/com/c2kernel/persistency/ProxyLoader.java +++ b/src/main/java/com/c2kernel/persistency/ProxyLoader.java @@ -11,6 +11,7 @@ import com.c2kernel.lookup.ItemPath; import com.c2kernel.lookup.Lookup; import com.c2kernel.persistency.outcome.Outcome; import com.c2kernel.process.Gateway; +import com.c2kernel.process.auth.Authenticator; import com.c2kernel.utils.Logger; /** Used by proxies to load clusters by queryData from the Entity. @@ -22,7 +23,7 @@ public class ProxyLoader extends ClusterStorage { Lookup lookup; @Override - public void open() throws ClusterStorageException { + public void open(Authenticator auth) throws ClusterStorageException { lookup = Gateway.getLookup(); } diff --git a/src/main/java/com/c2kernel/persistency/TransactionManager.java b/src/main/java/com/c2kernel/persistency/TransactionManager.java index d966eec..94b8123 100644 --- a/src/main/java/com/c2kernel/persistency/TransactionManager.java +++ b/src/main/java/com/c2kernel/persistency/TransactionManager.java @@ -7,6 +7,7 @@ import com.c2kernel.common.ObjectNotFoundException; import com.c2kernel.entity.C2KLocalObject; import com.c2kernel.entity.agent.JobList; import com.c2kernel.events.History; +import com.c2kernel.process.auth.Authenticator; import com.c2kernel.utils.Logger; public class TransactionManager { @@ -15,8 +16,8 @@ public class TransactionManager { HashMap> pendingTransactions; ClusterStorageManager storage; - public TransactionManager() throws ClusterStorageException { - storage = new ClusterStorageManager(); + public TransactionManager(Authenticator auth) throws ClusterStorageException { + storage = new ClusterStorageManager(auth); locks = new HashMap(); pendingTransactions = new HashMap>(); } diff --git a/src/main/java/com/c2kernel/persistency/XMLClusterStorage.java b/src/main/java/com/c2kernel/persistency/XMLClusterStorage.java index 50c76f0..e6c6e9f 100644 --- a/src/main/java/com/c2kernel/persistency/XMLClusterStorage.java +++ b/src/main/java/com/c2kernel/persistency/XMLClusterStorage.java @@ -3,10 +3,11 @@ import java.io.File; import java.util.ArrayList; import com.c2kernel.entity.C2KLocalObject; -import com.c2kernel.lookup.ItemPath; import com.c2kernel.lookup.InvalidItemPathException; +import com.c2kernel.lookup.ItemPath; import com.c2kernel.persistency.outcome.Outcome; import com.c2kernel.process.Gateway; +import com.c2kernel.process.auth.Authenticator; import com.c2kernel.utils.FileStringUtility; import com.c2kernel.utils.Logger; @@ -17,7 +18,7 @@ public class XMLClusterStorage extends ClusterStorage { } @Override - public void open() throws ClusterStorageException { + public void open(Authenticator auth) throws ClusterStorageException { String rootProp = Gateway.getProperties().getProperty("XMLStorage.root"); if (rootProp == null) throw new ClusterStorageException("XMLClusterStorage.open() - Root path not given in config file."); diff --git a/src/main/java/com/c2kernel/process/Gateway.java b/src/main/java/com/c2kernel/process/Gateway.java index 836b34b..2db7aa1 100644 --- a/src/main/java/com/c2kernel/process/Gateway.java +++ b/src/main/java/com/c2kernel/process/Gateway.java @@ -207,7 +207,7 @@ public class Gateway mLookup = (Lookup)mC2KProps.getInstance("Lookup"); mLookup.open(auth); - mStorage = new TransactionManager(); + mStorage = new TransactionManager(auth); mProxyManager = new ProxyManager(); } catch (Exception ex) { @@ -240,7 +240,7 @@ public class Gateway mLookup = (Lookup)mC2KProps.getInstance("Lookup"); mLookup.open(auth); - mStorage = new TransactionManager(); + mStorage = new TransactionManager(auth); mProxyManager = new ProxyManager(); // find agent proxy -- cgit v1.2.3