User: osh
Date: 00/09/11 22:04:49
Modified: src/main/org/jboss/ejb/plugins
EntitySynchronizationInterceptor.java
Log:
rollback bugfix and some cleanup
Revision Changes Path
1.14 +233 -230
jboss/src/main/org/jboss/ejb/plugins/EntitySynchronizationInterceptor.java
Index: EntitySynchronizationInterceptor.java
===================================================================
RCS file:
/products/cvs/ejboss/jboss/src/main/org/jboss/ejb/plugins/EntitySynchronizationInterceptor.java,v
retrieving revision 1.13
retrieving revision 1.14
diff -u -r1.13 -r1.14
--- EntitySynchronizationInterceptor.java 2000/08/25 17:51:37 1.13
+++ EntitySynchronizationInterceptor.java 2000/09/12 05:04:48 1.14
@@ -48,23 +48,43 @@
* @see <related>
* @author Rickard �berg ([EMAIL PROTECTED])
* @author <a href="mailto:[EMAIL PROTECTED]">Marc Fleury</a>
-* @version $Revision: 1.13 $
+* @version $Revision: 1.14 $
*/
public class EntitySynchronizationInterceptor
extends AbstractInterceptor
{
// Constants -----------------------------------------------------
- public static final int TIMEOUT = 10000;
- // Commit time options
+ /**
+ * Cache a "ready" instance between transactions.
+ * Data will <em>not</em> be reloaded from persistent storage when
+ * a new transaction is started.
+ * This option should only be used if the instance has exclusive
+ * access to its persistent storage.
+ */
public static final int A = 0; // Keep instance cached
+
+ /**
+ * Cache a "ready" instance between transactions and reload data
+ * from persistent storage on transaction start.
+ */
public static final int B = 1; // Invalidate state
+
+ /**
+ * Passivate instance after each transaction.
+ */
public static final int C = 2; // Passivate
// Attributes ----------------------------------------------------
- int commitOption = A;
-
+ /**
+ * The current commit option.
+ */
+ protected int commitOption = A;
+
+ /**
+ * The container of this interceptor.
+ */
protected EntityContainer container;
// Static --------------------------------------------------------
@@ -77,47 +97,60 @@
this.container = (EntityContainer)container;
}
- public Container getContainer()
+ public Container getContainer()
{
return container;
}
-
+
+ /**
+ * Setter for property commitOption.
+ */
public void setCommitOption(int commitOption)
{
this.commitOption = commitOption;
}
-
+
+ /**
+ * Getter for property commitOption.
+ */
public int getCommitOption()
{
return commitOption;
}
-
- public void register(EnterpriseContext ctx, Transaction tx)
+
+ /**
+ * Register a transaction synchronization callback with a context.
+ */
+ private void register(EntityEnterpriseContext ctx, Transaction tx)
{
// Create a new synchronization
- InstanceSynchronization synch = new InstanceSynchronization(tx);
-
+ InstanceSynchronization synch = new InstanceSynchronization(tx, ctx);
+
try {
-
- tx.registerSynchronization(synch);
- }
- catch (Exception e) {
-
- throw new EJBException(e);
+ // OSH: An extra check to avoid warning.
+ // Can go when we are sure that we no longer get
+ // the JTA violation warning.
+ if (tx.getStatus() == Status.STATUS_MARKED_ROLLBACK) {
+ ctx.setValid(false);
+ return;
+ }
+
+ tx.registerSynchronization(synch);
+ } catch (RollbackException e) {
+ ctx.setValid(false);
+ } catch (Exception e) {
+ throw new EJBException(e);
}
-
- // register
- synch.add(ctx);
}
-
- public void deregister(EntityEnterpriseContext ctx)
+
+ private void deregister(EntityEnterpriseContext ctx)
{
// MF FIXME: I suspect this is redundant now
// (won't the pool clean it up?)
// Deassociate ctx with tx
- ctx.setTransaction(null);
- ctx.setInvoked(false);
+ // OSH: TxInterceptor seems to do this: ctx.setTransaction(null);
+ // OSH: Pool seems to do this: ctx.setInvoked(false);
}
// Interceptor implementation --------------------------------------
@@ -125,25 +158,20 @@
public Object invokeHome(MethodInvocation mi)
throws Exception
{
- try
- {
- return getNext().invokeHome(mi);
- } finally
- {
- // Anonymous was sent in, so if it has an id it was created
- EnterpriseContext ctx = mi.getEnterpriseContext();
- if (ctx.getId() != null)
- {
- if (mi.getTransaction() != null &&
- mi.getTransaction().getStatus() == Status.STATUS_ACTIVE)
- {
- // Set tx
- register(ctx, mi.getTransaction());
- }
+ try {
+ return getNext().invokeHome(mi);
+ } finally {
+ // Anonymous was sent in, so if it has an id it was created
+ EntityEnterpriseContext ctx =
(EntityEnterpriseContext)mi.getEnterpriseContext();
+ if (ctx.getId() != null) {
+ Transaction tx = mi.getTransaction();
+
+ if (tx != null && tx.getStatus() == Status.STATUS_ACTIVE)
+ register(ctx, tx); // Set tx
- // Currently synched with underlying storage
- ((EntityEnterpriseContext)ctx).setValid(true);
- }
+ // Currently synched with underlying storage
+ ctx.setValid(true);
+ }
}
}
@@ -161,221 +189,196 @@
// Is my state valid?
if (!ctx.isValid()) {
-
- // If not tell the persistence manager to load the state
- ((EntityContainer)getContainer()).getPersistenceManager().loadEntity(ctx);
+ // If not tell the persistence manager to load the state
+ ((EntityContainer)getContainer()).getPersistenceManager().loadEntity(ctx);
- // Now the state is valid
- ctx.setValid(true);
+ // Now the state is valid
+ ctx.setValid(true);
}
// So we can go on with the invocation
Logger.log("Tx is "+ ((tx == null)? "null" : tx.toString()));
- if (tx != null &&
- tx.getStatus() == Status.STATUS_ACTIVE) {
-
- try {
-
- return getNext().invoke(mi);
- }
-
- finally {
-
- // Do we have a valid bean (not removed)
- if (ctx.getId() != null) {
-
- // If the context was not invoked previously...
- if (!ctx.isInvoked()) {
-
- // It is now and this will cause ejbStore to be called...
- ctx.setInvoked(true);
+ if (tx != null && tx.getStatus() == Status.STATUS_ACTIVE) {
+ try {
+ return getNext().invoke(mi);
+ } finally {
+ // Do we have a valid bean (not removed)
+ if (ctx.getId() != null) {
+ // If the context was not invoked previously...
+ if (!ctx.isInvoked()) {
+ // It is now and this will cause ejbStore to be called...
+ ctx.setInvoked(true);
- // ... on a transaction callback that we register here.
- register(ctx, tx);
- }
- }
- else {
-
- // Entity was removed
- if (ctx.getTransaction() != null) {
-
- // Disassociate ctx
- deregister(ctx);
- }
- }
+ // ... on a transaction callback that we register here.
+ register(ctx, tx);
+ }
+ } else // Entity was removed
+ if (ctx.getTransaction() != null)
+ deregister(ctx); // Disassociate ctx
- //Logger.debug("CTX out: isValid():"+ctx.isValid()+"
isInvoked():"+ctx.isInvoked());
- //Logger.debug("PresentTx:"+tx);
-
- }
- }
-
- else { // No tx
-
- try
- {
- Object result = getNext().invoke(mi);
-
- // Store after each invocation -- not on exception though, or removal
- // And skip reads too ("get" methods)
- if (ctx.getId() != null && !mi.getMethod().getName().startsWith("get"))
-
((EntityContainer)getContainer()).getPersistenceManager().storeEntity(ctx);
-
- return result;
- } catch (Exception e)
- {
- // Exception - force reload on next call
- ctx.setValid(false);
- throw e;
- }
+ //Logger.debug("CTX out: isValid():"+ctx.isValid()+"
isInvoked():"+ctx.isInvoked());
+ //Logger.debug("PresentTx:"+tx);
+ }
+ } else { // No tx
+ try {
+ Object result = getNext().invoke(mi);
+
+ // Store after each invocation -- not on exception though, or removal
+ // And skip reads too ("get" methods)
+ // OSH FIXME: Isn't this startsWith("get") optimization a violation of
+ // the EJB specification? Think of SequenceBean.getNext().
+ if (ctx.getId() != null && !mi.getMethod().getName().startsWith("get"))
+
((EntityContainer)getContainer()).getPersistenceManager().storeEntity(ctx);
+
+ return result;
+ } catch (Exception e) {
+ // Exception - force reload on next call
+ ctx.setValid(false);
+ throw e;
+ }
}
}
// Protected ----------------------------------------------------
// Inner classes -------------------------------------------------
- class InstanceSynchronization
+
+ private class InstanceSynchronization
implements Synchronization
{
- // The transaction we follow
- Transaction tx;
-
- // The context we manage
- EnterpriseContext ctx;
-
- InstanceSynchronization(Transaction tx)
+ /**
+ * The transaction we follow.
+ */
+ private Transaction tx;
+
+ /**
+ * The context we manage.
+ */
+ private EntityEnterpriseContext ctx;
+
+ /**
+ * Flag that we need to unlock context.
+ */
+ boolean lockedCtx = false;
+
+ /**
+ * Create a new instance synchronization instance.
+ */
+ InstanceSynchronization(Transaction tx, EntityEnterpriseContext ctx)
{
- this.tx = tx;
+ this.tx = tx;
+ this.ctx = ctx;
}
-
- public void add(EnterpriseContext ctx)
- {
- this.ctx = ctx;
- }
-
-
+
// Synchronization implementation -----------------------------
+
public void beforeCompletion()
{
-
- Logger.log("beforeCompletion called");
- try
- {
- try
- {
- // Lock instance
-
((EntityContainer)getContainer()).getInstanceCache().get(((EntityEnterpriseContext)
ctx).getCacheKey());
+ Logger.log("beforeCompletion called");
+ try {
+ try {
+ // Lock instance
+ container.getInstanceCache().get(ctx.getCacheKey());
+ lockedCtx = true;
- // Store instance if business method was
- if (((EntityEnterpriseContext) ctx).isInvoked())
-
((EntityContainer)getContainer()).getPersistenceManager().storeEntity((EntityEnterpriseContext)ctx);
-
- } catch (NoSuchEntityException e)
- {
- // Object has been removed -- ignore
+ // Store instance if business method was
+ if (ctx.isInvoked())
+ container.getPersistenceManager().storeEntity(ctx);
+ } catch (NoSuchEntityException e) {
+ // Object has been removed -- ignore
+ }
+ } catch (RemoteException e) {
+ // Store failed -> rollback!
+ try {
+ tx.setRollbackOnly();
+ } catch (SystemException ex) {
+ // DEBUG ex.printStackTrace();
+ }
}
- } catch (RemoteException e)
- {
- // Store failed -> rollback!
- try
- {
- tx.setRollbackOnly();
- } catch (SystemException ex)
- {
- // DEBUG ex.printStackTrace();
- }
- }
}
public void afterCompletion(int status)
{
- Logger.log("afterCompletion called");
- // If rolled back -> invalidate instance
- if (status == Status.STATUS_ROLLEDBACK)
- {
-
- try
- {
-
- ctx.setId(null);
- ctx.setTransaction(null);
- container.getInstanceCache().remove(ctx.getId());
- container.getInstancePool().free(ctx); // TODO: should this be done?
still valid instance?
- } catch (Exception e)
- {
- // Ignore
- }
- } else
- {
-
- // The transaction is done
- ctx.setTransaction(null);
-
- // We are afterCompletion so the invoked can be set to false (db sync is
done)
- ((EntityEnterpriseContext) ctx).setInvoked(false);
-
- switch (commitOption)
- {
- // Keep instance cached after tx commit
- case A:
- try
- {
- // The state is still valid (only point of access is us)
- ((EntityEnterpriseContext) ctx).setValid(true);
-
- // Release it though
- container.getInstanceCache().release(ctx);
- } catch (Exception e)
- {
- Logger.debug(e);
- }
- break;
-
- // Keep instance active, but invalidate state
- case B:
- try
- {
- // Invalidate state (there might be other points of entry)
- ((EntityEnterpriseContext) ctx).setValid(false);
-
- // Release it though
- container.getInstanceCache().release(ctx);
- } catch (Exception e)
- {
- Logger.debug(e);
- }
- break;
-
- // Invalidate everything AND Passivate instance
- case C:
- try
- {
- //Lock in cache
-
container.getInstanceCache().get(((EntityEnterpriseContext)ctx).getCacheKey());
-
- // Passivate instance
-
((EntityContainer)getContainer()).getPersistenceManager().passivateEntity((EntityEnterpriseContext)ctx);
-
- //Remove from the cache, it is not active anymore
- container.getInstanceCache().remove(ctx.getId());
-
- // Back to the pool
- container.getInstancePool().free(ctx);
-
- } catch (Exception e)
- {
- Logger.debug(e);
- }
- break;
-
+ Logger.log("afterCompletion called");
+ // If rolled back -> invalidate instance
+ if (status == Status.STATUS_ROLLEDBACK) {
+// OSH FIXME:
+// afterCompletion() was not called on rollback in with the old org.jboss.tm
+// package, but now it is. The code below, however, makes the bean have a
+// null primary key on the next call. Commenting it out as to get old behaviour.
+// What is the correct thing to do here?
+// try {
+// ctx.setId(null);
+// ctx.setTransaction(null);
+// container.getInstanceCache().remove(ctx.getId());
+// container.getInstancePool().free(ctx); // TODO: should this be
done? still valid instance?
+// } catch (Exception e) {
+// // Ignore
+// }
+// At least we should invalidate the state and release the context if it was locked.
+ ctx.setValid(false);
+ if (lockedCtx)
+ container.getInstanceCache().release(ctx);
+ } else {
+ // The transaction is done
+ ctx.setTransaction(null);
+
+ // We are afterCompletion so the invoked can be set to false (db sync
is done)
+ ctx.setInvoked(false);
+
+ switch (commitOption) {
+ // Keep instance cached after tx commit
+ case A:
+ try {
+ // The state is still valid (only point of access is us)
+ ctx.setValid(true);
+
+ // Release it though
+ container.getInstanceCache().release(ctx);
+ } catch (Exception e) {
+ Logger.debug(e);
+ }
+ break;
+
+ // Keep instance active, but invalidate state
+ case B:
+ try {
+ // Invalidate state (there might be other points of entry)
+ ctx.setValid(false);
+
+ // Release it though
+ container.getInstanceCache().release(ctx);
+ } catch (Exception e) {
+ Logger.debug(e);
+ }
+ break;
+
+ // Invalidate everything AND Passivate instance
+ case C:
+ try {
+ //Lock in cache
+ // OSH: Already did this in beforeCompletion():
container.getInstanceCache().get(ctx.getCacheKey());
+
+ // Passivate instance
+ container.getPersistenceManager().passivateEntity(ctx);
+
+ // Remove from the cache, it is not active anymore
+ container.getInstanceCache().remove(ctx.getId());
+
+ // Back to the pool
+ container.getInstancePool().free(ctx);
+ } catch (Exception e) {
+ Logger.debug(e);
+ }
+ break;
+ }
}
- }
- // Notify all who are waiting for this tx to end
- synchronized (tx)
- {
- tx.notifyAll();
- }
+ // OSH: What is this for? Who is waiting?
+ // Notify all who are waiting for this tx to end
+ synchronized (tx) {
+ tx.notifyAll();
+ }
}
}
}
