Permalink
Browse files

Fix deadlocks due to activation: db.activate() uses the db.lock(), so

we cannot use that for transaction-locking anymore since we have no
clear locking order policy for activate() - we now use our own
transaction lock.
  • Loading branch information...
xor-freenet committed May 19, 2011
1 parent 804c146 commit 94d7544e9aad7ace6cdcfe6ed57707e0c7b284a4
@@ -52,7 +52,7 @@ protected Configuration(WebOfTrust myWebOfTrust) {
* because the user interface will usually change many values at once.
*/
public synchronized void storeAndCommit() {
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
checkedActivate(4);
@@ -780,7 +780,7 @@ protected void storeWithoutCommit() {
*/
public final void storeAndCommit() {
synchronized(mWebOfTrust) {
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
storeWithoutCommit();
checkedCommit(this);
@@ -200,7 +200,7 @@ private IdentityFetcherCommand getCommand(final Class<? extends IdentityFetcherC
}
private synchronized void deleteAllCommands() {
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
if(logDEBUG) Logger.debug(this, "Deleting all identity fetcher commands ...");
@@ -302,7 +302,7 @@ public int getPriority() {
public void run() {
synchronized(mWoT) { // Lock needed because we do getIdentityByID() in fetch()
synchronized(this) {
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
if(logDEBUG) Logger.debug(this, "Processing identity fetcher commands ...");
@@ -53,6 +53,12 @@
*/
protected final Date mCreationDate = CurrentTimeUTC.get();
+ /**
+ * The object used for locking transactions.
+ * Since we only support one open database at a moment there is only one.
+ */
+ private static transient final Object mTransactionLock = new Object();
+
/* These booleans are used for preventing the construction of log-strings if logging is disabled (for saving some cpu cycles) */
protected static transient volatile boolean logDEBUG = false;
@@ -108,6 +114,18 @@ public final void initializeTransient(final WebOfTrust myWebOfTrust) {
mWebOfTrust = myWebOfTrust;
mDB = mWebOfTrust.getDatabase();
}
+
+ /**
+ * Returns the lock for creating a transaction.
+ * A proper transaction typically looks like this:
+ * synchronized(Persistent.transactionLock(db)) { try { ... do stuff ... Persistent.checkedCommit() } catch(RuntimeException e) { Persistent.checkedRollback(); } }
+ *
+ * The db parameter is currently ignored - the same lock will be returned for all databases!
+ * We don't need multi-database support in Freetalk yet.
+ */
+ public static final Object transactionLock(ExtObjectContainer db) {
+ return mTransactionLock;
+ }
/**
* Only to be used by the extending classes, not to be called from the outside.
@@ -215,7 +233,7 @@ public final void throwIfNotStored() {
/**
* This is one of the only functions which outside classes should use. Rolls back the current transaction, logs the passed exception and throws it.
* The call to this function must be embedded in a transaction, that is a block of:<br />
- * synchronized(mDB.lock()) {<br />
+ * synchronized(Persistent.transactionLock(mDB)) {<br />
* try { object.storeWithoutCommit(); object.checkedCommit(this); }<br />
* catch(RuntimeException e) { Persistent.checkedRollback(mDB, this, e); }<br />
* }
@@ -233,7 +251,7 @@ public static final void checkedRollback(final ExtObjectContainer db, final Obje
/**
* This is one of the only functions which outside classes should use. Rolls back the current transaction, logs the passed exception and throws it.
* The call to this function must be embedded in a transaction, that is a block of:<br />
- * synchronized(mDB.lock()) {<br />
+ * synchronized(Persistent.transactionLock(mDB)) {<br />
* try { object.storeWithoutCommit(); object.checkedCommit(this); }<br />
* catch(RuntimeException e) { Persistent.checkedRollbackAndThrow(mDB, this, e); }<br />
* }
@@ -275,7 +293,7 @@ protected void storeWithoutCommit(final int activationDepth) {
/**
* This is one of the only functions which outside classes should use. It is used for storing the object.
* The call to this function must be embedded in a transaction, that is a block of:<br />
- * synchronized(mDB.lock()) {<br />
+ * synchronized(Persistent.transactionLock(mDB)) {<br />
* try { object.storeWithoutCommit(); object.checkedCommit(this); }<br />
* catch(RuntimeException e) { Persistent.checkedRollbackAndThrow(mDB, this, e); }<br />
* }
@@ -307,7 +325,7 @@ protected void deleteWithoutCommit(final int activationDepth) {
/**
* This is one of the only functions which outside classes should use. It is used for deleting the object.
* The call to this function must be embedded in a transaction, that is a block of:<br />
- * synchronized(mDB.lock()) {<br />
+ * synchronized(Persistent.transactionLock(mDB)) {<br />
* try { object.storeWithoutCommit(); object.checkedCommit(this); }<br />
* catch(RuntimeException e) { Persistent.checkedRollbackAndThrow(mDB, this, e); }<br />
* }
@@ -320,7 +338,7 @@ protected void deleteWithoutCommit() {
/**
* This is one of the only functions which outside classes should use. It is used for committing the transaction.
* The call to this function must be embedded in a transaction, that is a block of:<br />
- * synchronized(mDB.lock()) {<br />
+ * synchronized(Persistent.transactionLock(mDB)) {<br />
* try { object.storeWithoutCommit(); Persistent.checkedCommit(mDB, this); }<br />
* catch(RuntimeException e) { Persistent.checkedRollbackAndThrow(mDB, this, e); }<br />
* }
@@ -335,7 +353,7 @@ public static final void checkedCommit(final ExtObjectContainer db, final Object
/**
* This is one of the only functions which outside classes should use. It is used for committing the transaction.
* The call to this function must be embedded in a transaction, that is a block of:<br />
- * synchronized(mDB.lock()) {<br />
+ * synchronized(Persistent.transactionLock(mDB)) {<br />
* try { object.storeWithoutCommit(); object.checkedCommit(this); }<br />
* catch(RuntimeException e) { Persistent.checkedRollbackAndThrow(mDB, this, e); }<br />
* }
@@ -202,7 +202,7 @@ public boolean realTimeFlag() {
// TODO: Don't do this as soon as we are sure that score computation works.
Logger.normal(this, "Veriying all stored scores ...");
synchronized(this) {
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
computeAllScoresWithoutCommit();
Persistent.checkedCommit(mDB, this);
@@ -404,7 +404,7 @@ private synchronized void verifyDatabaseIntegrity() {
* Debug function for deleting duplicate identities etc. which might have been created due to bugs :)
*/
private synchronized void deleteDuplicateObjects() {
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
HashSet<String> deleted = new HashSet<String>();
@@ -433,7 +433,7 @@ private synchronized void deleteDuplicateObjects() {
}
}
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
if(logDEBUG) Logger.debug(this, "Searching for duplicate Trust objects ...");
@@ -471,7 +471,7 @@ private synchronized void deleteDuplicateObjects() {
* Debug function for deleting trusts or scores of which one of the involved partners is missing.
*/
private synchronized void deleteOrphanObjects() {
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
boolean orphanTrustFound = false;
@@ -502,7 +502,7 @@ private synchronized void deleteOrphanObjects() {
}
}
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
boolean orphanScoresFound = false;
@@ -958,7 +958,7 @@ public void terminate() {
/* TODO: At 2009-06-15, it does not seem possible to ask db4o for whether a transaction is pending.
* If it becomes possible some day, we should check that here, and log an error if there is an uncommitted transaction.
* - All transactions should be committed after obtaining the lock() on the database. */
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
System.gc();
mDB.rollback();
System.gc();
@@ -1511,7 +1511,7 @@ else if(select < 0 )
* It creates or updates an existing Trust object and make the trustee compute its {@link Score}.
*
* This function does neither lock the database nor commit the transaction. You have to surround it with
- * synchronized(mDB.lock()) {
+ * synchronized(Persistent.transactionLock(mDB)) {
* try { ... setTrustWithoutCommit(...); mDB.commit(); }
* catch(RuntimeException e) { System.gc(); mDB.rollback(); throw e; }
* }
@@ -1555,7 +1555,7 @@ protected synchronized void setTrustWithoutCommit(Identity truster, Identity tru
synchronized void setTrust(OwnIdentity truster, Identity trustee, byte newValue, String newComment)
throws InvalidParameterException {
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
setTrustWithoutCommit(truster, trustee, newValue, newComment);
Persistent.checkedCommit(mDB, this);
@@ -1567,7 +1567,7 @@ synchronized void setTrust(OwnIdentity truster, Identity trustee, byte newValue,
}
protected synchronized void removeTrust(OwnIdentity truster, Identity trustee) {
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
removeTrustWithoutCommit(truster, trustee);
Persistent.checkedCommit(mDB, this);
@@ -1582,7 +1582,7 @@ protected synchronized void removeTrust(OwnIdentity truster, Identity trustee) {
* Deletes a trust object.
*
* This function does neither lock the database nor commit the transaction. You have to surround it with
- * synchronized(mDB.lock()) {
+ * synchronized(Persistent.transactionLock(mDB)) {
* try { ... removeTrustWithoutCommit(...); mDB.commit(); }
* catch(RuntimeException e) { System.gc(); mDB.rollback(); throw e; }
* }
@@ -1607,7 +1607,7 @@ protected synchronized void removeTrustWithoutCommit(OwnIdentity truster, Identi
/**
*
* This function does neither lock the database nor commit the transaction. You have to surround it with
- * synchronized(mDB.lock()) {
+ * synchronized(Persistent.transactionLock(mDB)) {
* try { ... setTrustWithoutCommit(...); mDB.commit(); }
* catch(RuntimeException e) { System.gc(); mDB.rollback(); throw e; }
* }
@@ -1625,7 +1625,7 @@ protected synchronized void removeTrustWithoutCommit(Trust trust) {
* The score will have a rank of 0, a capacity of 100 (= 100 percent) and a score value of Integer.MAX_VALUE.
*
* This function does neither lock the database nor commit the transaction. You have to surround it with
- * synchronized(mDB.lock()) {
+ * synchronized(Persistent.transactionLock(mDB)) {
* try { ... initTrustTreeWithoutCommit(...); mDB.commit(); }
* catch(RuntimeException e) { System.gc(); mDB.rollback(); throw e; }
* }
@@ -1742,7 +1742,7 @@ else if(rank == Integer.MAX_VALUE)
* This speeds up setTrust/removeTrust as the score calculation is only performed when endTrustListImport is called.
*
* You MUST synchronize on this WoT around beginTrustListImport, abortTrustListImport and finishTrustListImport!
- * You MUST create a database transaction by synchronizing on db.lock().
+ * You MUST create a database transaction by synchronizing on Persistent.transactionLock(db).
*/
protected void beginTrustListImport() {
if(mTrustListImportInProgress) {
@@ -1799,7 +1799,7 @@ protected void finishTrustListImport() {
* For understanding how score calculation works you should first read {@link computeAllScores
*
* This function does neither lock the database nor commit the transaction. You have to surround it with
- * synchronized(mDB.lock()) {
+ * synchronized(Persistent.transactionLock(mDB)) {
* try { ... updateScoreWithoutCommit(...); mDB.commit(); }
* catch(RuntimeException e) { System.gc(); mDB.rollback(); throw e; }
* }
@@ -1994,7 +1994,7 @@ public synchronized Identity addIdentity(String requestURI) throws MalformedURLE
public synchronized void deleteIdentity(Identity identity) {
synchronized(mPuzzleStore) {
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
deleteWithoutCommit(identity);
Persistent.checkedCommit(mDB, this);
@@ -2023,7 +2023,7 @@ public OwnIdentity createOwnIdentity(String nickName, boolean publishTrustList,
public synchronized OwnIdentity createOwnIdentity(String insertURI, String requestURI, String nickName,
boolean publishTrustList, String context) throws MalformedURLException, InvalidParameterException {
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
OwnIdentity identity;
try {
@@ -2077,7 +2077,7 @@ public synchronized OwnIdentity createOwnIdentity(String insertURI, String reque
public synchronized void restoreIdentity(String requestURI, String insertURI) throws MalformedURLException, InvalidParameterException {
OwnIdentity identity;
synchronized(mPuzzleStore) {
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
FreenetURI requestFreenetURI = new FreenetURI(requestURI);
FreenetURI insertFreenetURI = new FreenetURI(insertURI);
@@ -354,7 +354,7 @@ public void importIdentity(FreenetURI identityURI, InputStream xmlInputStream) t
throw xmlData.parseError;
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try { // Transaction rollback block
identity.setEdition(newEdition); // The identity constructor only takes the edition number as a hint, so we must store it explicitly.
boolean didPublishTrustListPreviously = identity.doesPublishTrustList();
@@ -464,7 +464,7 @@ public void importIdentity(FreenetURI identityURI, InputStream xmlInputStream) t
mWoT.abortTrustListImport(e); // Does the rollback
throw e;
} // try
- } // synchronized(db.lock())
+ } // synchronized(Persistent.transactionLock(db))
Logger.normal(this, "Finished XML import for " + identity);
} // synchronized(mWoT)
@@ -568,7 +568,7 @@ public Identity importIntroduction(OwnIdentity puzzleOwner, InputStream xmlInput
if(!puzzleOwner.hasContext(IntroductionPuzzle.INTRODUCTION_CONTEXT))
throw new InvalidParameterException("Trying to import an identity identroduction for an own identity which does not allow introduction.");
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
try {
newIdentity = mWoT.getIdentityByURI(identityURI);
@@ -108,7 +108,7 @@ protected synchronized void deleteExpiredPuzzles() {
int deleted = 0;
for(IntroductionPuzzle p : result) {
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
if(logDEBUG) Logger.debug(this, "Deleting expired puzzle, was valid until " + p.getValidUntilDate());
p.deleteWithoutCommit();
@@ -149,7 +149,7 @@ protected synchronized void deleteOldestUnsolvedPuzzles(final int puzzlePoolSize
while(deleteCount > 0 && result.hasNext()) {
final IntroductionPuzzle puzzle = result.next();
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
puzzle.deleteWithoutCommit();
Persistent.checkedCommit(mDB, this);
@@ -193,7 +193,7 @@ public synchronized void storeAndCommit(final IntroductionPuzzle puzzle) {
/* TODO: Convert to assert() maybe when we are sure that this does not happen. Duplicate puzzles will be deleted after they
* expire anyway. Further, isn't there a db4o option which ensures that mID is a primary key and therefore no duplicates can exist? */
synchronized(puzzle) {
- synchronized(mDB.lock()) {
+ synchronized(Persistent.transactionLock(mDB)) {
try {
final IntroductionPuzzle existingPuzzle = getByID(puzzle.getID());
if(existingPuzzle != puzzle)

0 comments on commit 94d7544

Please sign in to comment.