[Mulgara-svn] r427 - branches/mgr-70/src/jar/resolver/java/org/mulgara/resolver
andrae at mulgara.org
andrae at mulgara.org
Wed Sep 12 04:39:45 UTC 2007
Author: andrae
Date: 2007-09-11 23:39:45 -0500 (Tue, 11 Sep 2007)
New Revision: 427
Modified:
branches/mgr-70/src/jar/resolver/java/org/mulgara/resolver/MulgaraTransaction.java
branches/mgr-70/src/jar/resolver/java/org/mulgara/resolver/MulgaraTransactionManager.java
Log:
Due to the use of a synchronized method, the logic that detected concurrent
access to a MulgaraTransaction was inside the critical region. This meant that
if one thread was using the transaction, and other attempted to access it via
the transaction-manager there could be a lock-order inversion between the
manager's mutex and the transaction's monitor.
Removed the implicit synchronization; replaced with an explicit reentrant lock;
protected the concurrent access detection logic within a seperate mutex held
only for the duration of access to the field.
This should mean that the concurrent access from the transaction-manager should
be detected as concurrent and throw the appropriate exception _before_
attempting to obtain a lock on the transaction.
Modified: branches/mgr-70/src/jar/resolver/java/org/mulgara/resolver/MulgaraTransaction.java
===================================================================
--- branches/mgr-70/src/jar/resolver/java/org/mulgara/resolver/MulgaraTransaction.java 2007-09-12 01:11:57 UTC (rev 426)
+++ branches/mgr-70/src/jar/resolver/java/org/mulgara/resolver/MulgaraTransaction.java 2007-09-12 04:39:45 UTC (rev 427)
@@ -19,6 +19,7 @@
// Java 2 enterprise packages
import java.util.HashSet;
import java.util.Set;
+import java.util.concurrent.locks.ReentrantLock;
import javax.transaction.RollbackException;
import javax.transaction.SystemException;
import javax.transaction.Transaction;
@@ -88,6 +89,8 @@
private Transaction transaction;
private Thread currentThread;
+ private ReentrantLock activationMutex;
+
private State state;
private int inuse;
private int using;
@@ -107,6 +110,8 @@
this.manager = manager;
this.context = context;
this.enlisted = new HashSet<EnlistableResource>();
+ this.activationMutex = new ReentrantLock();
+ this.currentThread = null;
inuse = 0;
using = 0;
@@ -118,49 +123,55 @@
}
- synchronized void activate() throws MulgaraTransactionException {
+ void activate() throws MulgaraTransactionException {
// report("Activating Transaction");
-
try {
- if (currentThread != null && !currentThread.equals(Thread.currentThread())) {
- throw new MulgaraTransactionException("Concurrent access attempted to transaction: Transaction has NOT been rolledback.");
+ synchronized (this) {
+ if (currentThread != null && !currentThread.equals(Thread.currentThread())) {
+ throw new MulgaraTransactionException("Concurrent access attempted to transaction: Transaction has NOT been rolledback.");
+ }
}
-
- switch (state) {
- case CONSTRUCTEDUNREF:
- startTransaction();
- inuse = 1;
- state = State.ACTUNREF;
- try {
- context.initiate(this);
- } catch (Throwable th) {
- throw implicitRollback(th);
- }
- break;
- case CONSTRUCTEDREF:
- startTransaction();
- inuse = 1;
- using = 1;
- state = State.ACTREF;
- try {
- context.initiate(this);
- } catch (Throwable th) {
- throw implicitRollback(th);
- }
- break;
- case DEACTREF:
- resumeTransaction();
- inuse = 1;
- state = State.ACTREF;
- break;
- case ACTREF:
- case ACTUNREF:
- inuse++;
- break;
- case FINISHED:
- throw new MulgaraTransactionException("Attempt to activate terminated transaction");
- case FAILED:
- throw new MulgaraTransactionException("Attempt to activate failed transaction", rollbackCause);
+
+ acquireActivationMutex();
+ try {
+ switch (state) {
+ case CONSTRUCTEDUNREF:
+ startTransaction();
+ inuse = 1;
+ state = State.ACTUNREF;
+ try {
+ context.initiate(this);
+ } catch (Throwable th) {
+ throw implicitRollback(th);
+ }
+ break;
+ case CONSTRUCTEDREF:
+ startTransaction();
+ inuse = 1;
+ using = 1;
+ state = State.ACTREF;
+ try {
+ context.initiate(this);
+ } catch (Throwable th) {
+ throw implicitRollback(th);
+ }
+ break;
+ case DEACTREF:
+ resumeTransaction();
+ inuse = 1;
+ state = State.ACTREF;
+ break;
+ case ACTREF:
+ case ACTUNREF:
+ inuse++;
+ break;
+ case FINISHED:
+ throw new MulgaraTransactionException("Attempt to activate terminated transaction");
+ case FAILED:
+ throw new MulgaraTransactionException("Attempt to activate failed transaction", rollbackCause);
+ }
+ } finally {
+ releaseActivationMutex();
}
try {
@@ -178,45 +189,52 @@
}
- private synchronized void deactivate() throws MulgaraTransactionException {
+ private void deactivate() throws MulgaraTransactionException {
// report("Deactivating transaction");
try {
- if (currentThread == null) {
- throw new MulgaraTransactionException("Transaction not associated with thread");
- } else if (!currentThread.equals(Thread.currentThread())) {
- throw new MulgaraTransactionException("Concurrent access attempted to transaction: Transaction has NOT been rolledback.");
+ synchronized (this) {
+ if (currentThread == null) {
+ throw new MulgaraTransactionException("Transaction not associated with thread");
+ } else if (!currentThread.equals(Thread.currentThread())) {
+ throw new MulgaraTransactionException("Concurrent access attempted to transaction: Transaction has NOT been rolledback.");
+ }
}
-
- switch (state) {
- case ACTUNREF:
- if (inuse == 1) {
- commitTransaction();
- }
- inuse--;
- break;
- case ACTREF:
- if (inuse == 1) {
- suspendTransaction();
- }
- inuse--;
- break;
- case CONSTRUCTEDREF:
- throw new MulgaraTransactionException("Attempt to deactivate uninitiated refed transaction");
- case CONSTRUCTEDUNREF:
- throw new MulgaraTransactionException("Attempt to deactivate uninitiated transaction");
- case DEACTREF:
- throw new IllegalStateException("Attempt to deactivate unactivated transaction");
- case FINISHED:
- if (inuse < 0) {
- errorReport("Activation count failure - too many deacts - in finished transaction", null);
- } else {
+
+ acquireActivationMutex();
+ try {
+ switch (state) {
+ case ACTUNREF:
+ if (inuse == 1) {
+ commitTransaction();
+ }
inuse--;
- }
- break;
- case FAILED:
- // Nothing to do here.
- break;
+ break;
+ case ACTREF:
+ if (inuse == 1) {
+ suspendTransaction();
+ }
+ inuse--;
+ break;
+ case CONSTRUCTEDREF:
+ throw new MulgaraTransactionException("Attempt to deactivate uninitiated refed transaction");
+ case CONSTRUCTEDUNREF:
+ throw new MulgaraTransactionException("Attempt to deactivate uninitiated transaction");
+ case DEACTREF:
+ throw new IllegalStateException("Attempt to deactivate unactivated transaction");
+ case FINISHED:
+ if (inuse < 0) {
+ errorReport("Activation count failure - too many deacts - in finished transaction", null);
+ } else {
+ inuse--;
+ }
+ break;
+ case FAILED:
+ // Nothing to do here.
+ break;
+ }
+ } finally {
+ releaseActivationMutex();
}
} catch (MulgaraTransactionException em) {
throw em;
@@ -235,9 +253,12 @@
try {
report("Referencing Transaction");
- if (currentThread != null && !currentThread.equals(Thread.currentThread())) {
- throw new MulgaraTransactionException("Concurrent access attempted to transaction: Transaction has NOT been rolledback.");
+ synchronized (this) {
+ if (currentThread != null && !currentThread.equals(Thread.currentThread())) {
+ throw new MulgaraTransactionException("Concurrent access attempted to transaction: Transaction has NOT been rolledback.");
+ }
}
+
switch (state) {
case CONSTRUCTEDUNREF:
state = State.CONSTRUCTEDREF;
@@ -270,9 +291,12 @@
void dereference() throws MulgaraTransactionException {
report("Dereferencing Transaction");
try {
- if (currentThread != null && !currentThread.equals(Thread.currentThread())) {
- throw new MulgaraTransactionException("Concurrent access attempted to transaction: Transaction has NOT been rolledback.");
+ synchronized (this) {
+ if (currentThread != null && !currentThread.equals(Thread.currentThread())) {
+ throw new MulgaraTransactionException("Concurrent access attempted to transaction: Transaction has NOT been rolledback.");
+ }
}
+
switch (state) {
case ACTREF:
if (using == 1) {
@@ -311,7 +335,9 @@
report("Initiating transaction");
try {
transaction = manager.transactionStart(this);
- currentThread = Thread.currentThread();
+ synchronized (this) {
+ currentThread = Thread.currentThread();
+ }
} catch (Throwable th) {
throw abortTransaction("Failed to start transaction", th);
}
@@ -321,7 +347,9 @@
// report("Resuming transaction");
try {
manager.transactionResumed(this, transaction);
- currentThread = Thread.currentThread();
+ synchronized (this) {
+ currentThread = Thread.currentThread();
+ }
} catch (Throwable th) {
abortTransaction("Failed to resume transaction", th);
}
@@ -335,7 +363,9 @@
new MulgaraTransactionException("Attempt to suspend unreferenced transaction"));
}
transaction = manager.transactionSuspended(this);
- currentThread = null;
+ synchronized (this) {
+ currentThread = null;
+ }
state = State.DEACTREF;
} catch (Throwable th) {
throw implicitRollback(th);
@@ -364,9 +394,11 @@
manager.transactionComplete(this);
} finally { try {
manager = null;
+ } finally { try {
+ currentThread = null;
} finally {
report("Committed transaction");
- } } } } } }
+ } } } } } } }
} catch (Throwable th) {
errorReport("Error cleaning up transaction post-commit", th);
throw new MulgaraTransactionException("Error cleaning up transaction post-commit", th);
@@ -379,10 +411,12 @@
* after all we requested it.
*/
public void explicitRollback() throws MulgaraTransactionException {
- if (currentThread == null) {
- throw new MulgaraTransactionException("Transaction failed activation check");
- } else if (!currentThread.equals(Thread.currentThread())) {
- throw new MulgaraTransactionException("Concurrent access attempted to transaction: Transaction has NOT been rolledback.");
+ synchronized (this) {
+ if (currentThread == null) {
+ throw new MulgaraTransactionException("Transaction failed activation check");
+ } else if (!currentThread.equals(Thread.currentThread())) {
+ throw new MulgaraTransactionException("Concurrent access attempted to transaction: Transaction has NOT been rolledback.");
+ }
}
try {
@@ -426,10 +460,12 @@
try {
report("Implicit Rollback triggered");
- if (currentThread == null) {
- throw new MulgaraTransactionException("Transaction not associated with thread");
- } else if (!currentThread.equals(Thread.currentThread())) {
- throw new MulgaraTransactionException("Concurrent access attempted to transaction: Transaction has NOT been rolledback.");
+ synchronized (this) {
+ if (currentThread == null) {
+ throw new MulgaraTransactionException("Transaction not associated with thread");
+ } else if (!currentThread.equals(Thread.currentThread())) {
+ throw new MulgaraTransactionException("Concurrent access attempted to transaction: Transaction has NOT been rolledback.");
+ }
}
if (rollbackCause != null) {
@@ -450,6 +486,7 @@
state = State.FAILED;
manager.transactionComplete(this);
manager = null;
+ currentThread = null;
return new MulgaraTransactionException("Transaction rollback triggered", cause);
case DEACTREF:
throw new IllegalStateException("Attempt to rollback deactivated transaction");
@@ -508,9 +545,11 @@
transaction = null;
} finally { try {
manager = null;
+ } finally { try {
+ state = State.FAILED;
} finally {
- state = State.FAILED;
- } } } } } } } }
+ currentThread = null;
+ } } } } } } } } }
return new MulgaraTransactionException(errorMessage + " - Aborting", cause);
} catch (Throwable th) {
throw new MulgaraTransactionException(errorMessage + " - Failed to abort cleanly", th);
@@ -593,10 +632,12 @@
public void enlist(EnlistableResource enlistable) throws MulgaraTransactionException {
try {
- if (currentThread == null) {
- throw new MulgaraTransactionException("Transaction not associated with thread");
- } else if (!currentThread.equals(Thread.currentThread())) {
- throw new MulgaraTransactionException("Concurrent access attempted to transaction: Transaction has NOT been rolledback.");
+ synchronized (this) {
+ if (currentThread == null) {
+ throw new MulgaraTransactionException("Transaction not associated with thread");
+ } else if (!currentThread.equals(Thread.currentThread())) {
+ throw new MulgaraTransactionException("Concurrent access attempted to transaction: Transaction has NOT been rolledback.");
+ }
}
if (enlisted.contains(enlistable)) {
@@ -630,32 +671,42 @@
//
private void checkActivated() throws MulgaraTransactionException {
- if (currentThread == null) {
- throw new MulgaraTransactionException("Transaction not associated with thread");
- } else if (!currentThread.equals(Thread.currentThread())) {
- throw new MulgaraTransactionException("Concurrent access attempted to transaction: Transaction has NOT been rolledback.");
- } else {
- switch (state) {
- case ACTUNREF:
- case ACTREF:
- if (inuse < 0 || using < 0) {
- throw new MulgaraTransactionException("Reference Failure, using: " + using + ", inuse: " + inuse);
- }
- return;
- case CONSTRUCTEDREF:
- throw new MulgaraTransactionException("Transaction (ref) uninitiated");
- case CONSTRUCTEDUNREF:
- throw new MulgaraTransactionException("Transaction (unref) uninitiated");
- case DEACTREF:
- throw new MulgaraTransactionException("Transaction deactivated");
- case FINISHED:
- throw new MulgaraTransactionException("Transaction is terminated");
- case FAILED:
- throw new MulgaraTransactionException("Transaction is failed", rollbackCause);
+ synchronized (this) {
+ if (currentThread == null) {
+ throw new MulgaraTransactionException("Transaction not associated with thread");
+ } else if (!currentThread.equals(Thread.currentThread())) {
+ throw new MulgaraTransactionException("Concurrent access attempted to transaction: Transaction has NOT been rolledback.");
}
}
+
+ switch (state) {
+ case ACTUNREF:
+ case ACTREF:
+ if (inuse < 0 || using < 0) {
+ throw new MulgaraTransactionException("Reference Failure, using: " + using + ", inuse: " + inuse);
+ }
+ return;
+ case CONSTRUCTEDREF:
+ throw new MulgaraTransactionException("Transaction (ref) uninitiated");
+ case CONSTRUCTEDUNREF:
+ throw new MulgaraTransactionException("Transaction (unref) uninitiated");
+ case DEACTREF:
+ throw new MulgaraTransactionException("Transaction deactivated");
+ case FINISHED:
+ throw new MulgaraTransactionException("Transaction is terminated");
+ case FAILED:
+ throw new MulgaraTransactionException("Transaction is failed", rollbackCause);
+ }
}
+ private void acquireActivationMutex() {
+ activationMutex.lock();
+ }
+
+ private void releaseActivationMutex() {
+ activationMutex.unlock();
+ }
+
protected void finalize() {
report("GC-finalize");
if (state != State.FINISHED && state != State.FAILED) {
@@ -668,7 +719,7 @@
}
if (state != State.FAILED && (inuse != 0 || using != 0)) {
- errorReport("Referernce counting error in transaction", null);
+ errorReport("Reference counting error in transaction", null);
}
if (manager != null || transaction != null) {
Modified: branches/mgr-70/src/jar/resolver/java/org/mulgara/resolver/MulgaraTransactionManager.java
===================================================================
--- branches/mgr-70/src/jar/resolver/java/org/mulgara/resolver/MulgaraTransactionManager.java 2007-09-12 01:11:57 UTC (rev 426)
+++ branches/mgr-70/src/jar/resolver/java/org/mulgara/resolver/MulgaraTransactionManager.java 2007-09-12 04:39:45 UTC (rev 427)
@@ -528,10 +528,18 @@
}
}
+ /**
+ * Used to replace the built in monitor to allow it to be properly released
+ * during potentially blocking operations. All potentially blocking
+ * operations involve writes, so in these cases the write-lock is reserved
+ * allowing the mutex to be safely released and then reobtained after the
+ * blocking operation concludes.
+ */
private void acquireMutex() {
mutex.lock();
}
+
/**
* Used to reserve the write lock during a commit or rollback.
*/
More information about the Mulgara-svn
mailing list