[infinispan-commits] Infinispan SVN: r2165 - in branches/4.1.x/core/src: main/java/org/infinispan/interceptors and 2 other directories.

infinispan-commits at lists.jboss.org infinispan-commits at lists.jboss.org
Wed Aug 4 13:21:26 EDT 2010


Author: manik.surtani at jboss.com
Date: 2010-08-04 13:21:25 -0400 (Wed, 04 Aug 2010)
New Revision: 2165

Modified:
   branches/4.1.x/core/src/main/java/org/infinispan/commands/control/LockControlCommand.java
   branches/4.1.x/core/src/main/java/org/infinispan/interceptors/MarshalledValueInterceptor.java
   branches/4.1.x/core/src/main/java/org/infinispan/util/concurrent/locks/LockManager.java
   branches/4.1.x/core/src/test/java/org/infinispan/context/MarshalledValueContextTest.java
Log:
[ISPN-570] (Locks not properly cleaned up when using explicit lock API, marshalled values and custom key types)

Modified: branches/4.1.x/core/src/main/java/org/infinispan/commands/control/LockControlCommand.java
===================================================================
--- branches/4.1.x/core/src/main/java/org/infinispan/commands/control/LockControlCommand.java	2010-08-04 17:15:42 UTC (rev 2164)
+++ branches/4.1.x/core/src/main/java/org/infinispan/commands/control/LockControlCommand.java	2010-08-04 17:21:25 UTC (rev 2165)
@@ -31,8 +31,13 @@
 import org.infinispan.marshall.exts.ReplicableCommandExternalizer;
 import org.infinispan.transaction.xa.GlobalTransaction;
 import org.infinispan.transaction.xa.RemoteTransaction;
+import org.infinispan.util.Util;
 
 import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
 
 /**
  * LockControlCommand is a command that enables distributed locking across infinispan nodes.
@@ -46,19 +51,30 @@
 @Marshallable(externalizer = ReplicableCommandExternalizer.class, id = Ids.LOCK_CONTROL_COMMAND)
 public class LockControlCommand extends AbstractTransactionBoundaryCommand {
    public static final int COMMAND_ID = 3;
-   private Collection keys;
+   private Set<Object> keys;
+   private Object singleKey;
    private boolean implicit = false;
 
    public LockControlCommand() {
    }
 
-   public LockControlCommand(Collection keys, String cacheName) {
+   public LockControlCommand(Collection<Object> keys, String cacheName) {
       this(keys, cacheName, false);
    }
 
-   public LockControlCommand(Collection keys, String cacheName, boolean implicit) {
+   public LockControlCommand(Collection<Object> keys, String cacheName, boolean implicit) {
       this.cacheName = cacheName;
-      this.keys = keys;
+      this.keys = null;
+      this.singleKey = null;
+      if (keys != null && !keys.isEmpty()) {
+         if (keys.size() == 1) {
+            for (Object k: keys) this.singleKey = k;
+         } else {
+            // defensive copy
+            this.keys = new HashSet<Object>(keys);
+         }
+
+      }
       this.implicit = implicit;
    }
 
@@ -66,10 +82,48 @@
       globalTx = gtx;
    }
 
-   public Collection getKeys() {
+   public Set<Object> getKeys() {
+      if (keys == null) {
+         if (singleKey == null)
+            return Collections.emptySet();
+         else
+            return Collections.singleton(singleKey);
+      }
+
       return keys;
    }
 
+   public void replaceKey(Object oldKey, Object replacement) {
+      if (singleKey != null && singleKey.equals(oldKey)) {
+         singleKey = replacement;
+      } else {
+         if (keys != null) {
+            if (keys.remove(oldKey)) keys.add(replacement);
+         }
+      }
+   }
+
+   public void replaceKeys(Map<Object, Object> replacements) {
+      for (Map.Entry<Object, Object> e: replacements.entrySet()) replaceKey(e.getKey(), e.getValue());
+   }
+
+   public boolean multipleKeys() {
+      return keys != null && keys.size() > 1;
+   }
+
+   public Object getSingleKey() {
+      if (singleKey == null) {
+         if (keys != null) {
+            for (Object sk: keys) return sk;
+            return null;
+         } else {
+            return null;
+         }
+      } else {
+         return singleKey;
+      }
+   }
+
    public boolean isImplicit() {
       return implicit;
    }
@@ -104,15 +158,35 @@
    }
 
    public Object[] getParameters() {
-      return new Object[]{globalTx, cacheName, keys};
+      if (keys == null || keys.isEmpty()) {
+         if (singleKey == null)
+            return new Object[]{globalTx, cacheName, (byte) 1};
+         else
+            return new Object[]{globalTx, cacheName, (byte) 2, singleKey};
+      }
+      return new Object[]{globalTx, cacheName, (byte) 3, keys};
    }
 
+   @SuppressWarnings("unchecked")
    public void setParameters(int commandId, Object[] args) {
       if (commandId != COMMAND_ID)
          throw new IllegalStateException("Unusupported command id:" + commandId);
       globalTx = (GlobalTransaction) args[0];
       cacheName = (String) args[1];
-      keys = (Collection) args[2];
+
+      keys = null;
+      singleKey = null;
+      byte mode = (Byte) args[2];
+      switch (mode) {
+         case 1:
+            break; // do nothing
+         case 2:
+            singleKey = args[3];
+            break;
+         case 3:
+            keys = (Set<Object>) args[3];
+            break;
+      }
    }
 
    public boolean equals(Object o) {
@@ -124,12 +198,13 @@
       LockControlCommand that = (LockControlCommand) o;
       if (!super.equals(that))
          return false;
-      return keys.equals(that.getKeys());
+      return keys.equals(that.keys) && Util.safeEquals(singleKey, that.singleKey);
    }
 
    public int hashCode() {
       int result = super.hashCode();
-      return 31 * result + (keys != null ? keys.hashCode() : 0);
+      result = 31 * result + (keys != null ? keys.hashCode() : 0);
+      return 31 * result + (singleKey != null ? singleKey.hashCode() : 0);
    }
 
    @Override
@@ -138,6 +213,7 @@
             "gtx=" + globalTx +
             ", cacheName='" + cacheName +
             ", implicit='" + implicit +
-            ", keys=" + keys + '}';
+            ", keys=" + keys +
+            ", singleKey=" + singleKey + '}';
    }
 }

Modified: branches/4.1.x/core/src/main/java/org/infinispan/interceptors/MarshalledValueInterceptor.java
===================================================================
--- branches/4.1.x/core/src/main/java/org/infinispan/interceptors/MarshalledValueInterceptor.java	2010-08-04 17:15:42 UTC (rev 2164)
+++ branches/4.1.x/core/src/main/java/org/infinispan/interceptors/MarshalledValueInterceptor.java	2010-08-04 17:21:25 UTC (rev 2165)
@@ -21,6 +21,7 @@
  */
 package org.infinispan.interceptors;
 
+import org.infinispan.commands.control.LockControlCommand;
 import org.infinispan.commands.read.EntrySetCommand;
 import org.infinispan.commands.read.GetKeyValueCommand;
 import org.infinispan.commands.read.KeySetCommand;
@@ -32,6 +33,7 @@
 import org.infinispan.container.entries.InternalCacheEntry;
 import org.infinispan.container.entries.InternalEntryFactory;
 import org.infinispan.context.InvocationContext;
+import org.infinispan.context.impl.TxInvocationContext;
 import org.infinispan.factories.annotations.Inject;
 import org.infinispan.interceptors.base.CommandInterceptor;
 import org.infinispan.marshall.MarshalledValue;
@@ -48,10 +50,12 @@
 import java.util.Map;
 import java.util.Set;
 
+import static org.infinispan.marshall.MarshalledValue.isTypeExcluded;
+
 /**
  * Interceptor that handles the wrapping and unwrapping of cached data using {@link
- * org.infinispan.marshall.MarshalledValue}s. Known "excluded" types are not wrapped/unwrapped, which at this time include
- * {@link String}, Java primitives and their Object wrappers, as well as arrays of excluded types.
+ * org.infinispan.marshall.MarshalledValue}s. Known "excluded" types are not wrapped/unwrapped, which at this time
+ * include {@link String}, Java primitives and their Object wrappers, as well as arrays of excluded types.
  * <p/>
  * The {@link org.infinispan.marshall.MarshalledValue} wrapper handles lazy deserialization from byte array
  * representations.
@@ -64,13 +68,31 @@
  */
 public class MarshalledValueInterceptor extends CommandInterceptor {
    private StreamingMarshaller marshaller;
-   
+
    @Inject
    protected void injectMarshaller(StreamingMarshaller marshaller) {
       this.marshaller = marshaller;
    }
-   
+
    @Override
+   public Object visitLockControlCommand(TxInvocationContext ctx, LockControlCommand command) throws Throwable {
+      if (command.multipleKeys()) {
+         Collection<Object> rawKeys = command.getKeys();
+         Map<Object, Object> keyToMarshalledKeyMapping = new HashMap<Object, Object>(rawKeys.size());
+         for (Object k : rawKeys) {
+            if (!isTypeExcluded(k.getClass())) keyToMarshalledKeyMapping.put(k, createMarshalledValue(k, ctx));
+         }
+
+         if (!keyToMarshalledKeyMapping.isEmpty()) command.replaceKeys(keyToMarshalledKeyMapping);
+      } else {
+         Object key = command.getSingleKey();
+         if (!isTypeExcluded(key.getClass())) command.replaceKey(key, createMarshalledValue(key, ctx));
+      }
+
+      return invokeNextInterceptor(ctx, command);
+   }
+
+   @Override
    public Object visitPutMapCommand(InvocationContext ctx, PutMapCommand command) throws Throwable {
       Set<MarshalledValue> marshalledValues = new HashSet<MarshalledValue>();
       Map map = wrapMap(command.getMap(), marshalledValues, ctx);
@@ -83,11 +105,11 @@
    public Object visitPutKeyValueCommand(InvocationContext ctx, PutKeyValueCommand command) throws Throwable {
       MarshalledValue key = null;
       MarshalledValue value = null;
-      if (!MarshalledValue.isTypeExcluded(command.getKey().getClass())) {
+      if (!isTypeExcluded(command.getKey().getClass())) {
          key = createMarshalledValue(command.getKey(), ctx);
          command.setKey(key);
       }
-      if (!MarshalledValue.isTypeExcluded(command.getValue().getClass())) {
+      if (!isTypeExcluded(command.getValue().getClass())) {
          value = createMarshalledValue(command.getValue(), ctx);
          command.setValue(value);
       }
@@ -100,7 +122,7 @@
    @Override
    public Object visitRemoveCommand(InvocationContext ctx, RemoveCommand command) throws Throwable {
       MarshalledValue value = null;
-      if (!MarshalledValue.isTypeExcluded(command.getKey().getClass())) {
+      if (!isTypeExcluded(command.getKey().getClass())) {
          value = createMarshalledValue(command.getKey(), ctx);
          command.setKey(value);
       }
@@ -108,11 +130,11 @@
       compact(value);
       return processRetVal(retVal);
    }
-   
+
    @Override
    public Object visitEvictCommand(InvocationContext ctx, org.infinispan.commands.write.EvictCommand command) throws Throwable {
       MarshalledValue value = null;
-      if (!MarshalledValue.isTypeExcluded(command.getKey().getClass())) {
+      if (!isTypeExcluded(command.getKey().getClass())) {
          value = createMarshalledValue(command.getKey(), ctx);
          command.setKey(value);
       }
@@ -124,7 +146,7 @@
    @Override
    public Object visitGetKeyValueCommand(InvocationContext ctx, GetKeyValueCommand command) throws Throwable {
       MarshalledValue mv = null;
-      if (!MarshalledValue.isTypeExcluded(command.getKey().getClass())) {
+      if (!isTypeExcluded(command.getKey().getClass())) {
          mv = createMarshalledValue(command.getKey(), ctx);
          command.setKey(mv);
          compact(mv);
@@ -133,7 +155,7 @@
       compact(mv);
       return processRetVal(retVal);
    }
-   
+
    @Override
    public Object visitKeySetCommand(InvocationContext ctx, KeySetCommand command) throws Throwable {
       Set keys = (Set) invokeNextInterceptor(ctx, command);
@@ -141,7 +163,7 @@
       for (Object key : keys) {
          if (key instanceof MarshalledValue) {
             key = ((MarshalledValue) key).get();
-         } 
+         }
          copy.add(key);
       }
       return Immutables.immutableSetWrap(copy);
@@ -150,7 +172,7 @@
    @Override
    public Object visitValuesCommand(InvocationContext ctx, ValuesCommand command) throws Throwable {
       Collection values = (Collection) invokeNextInterceptor(ctx, command);
-      Collection copy = new ArrayList();  
+      Collection copy = new ArrayList();
       for (Object value : values) {
          if (value instanceof MarshalledValue) {
             value = ((MarshalledValue) value).get();
@@ -159,7 +181,7 @@
       }
       return Immutables.immutableCollectionWrap(copy);
    }
-   
+
    @Override
    public Object visitEntrySetCommand(InvocationContext ctx, EntrySetCommand command) throws Throwable {
       Set<InternalCacheEntry> entries = (Set<InternalCacheEntry>) invokeNextInterceptor(ctx, command);
@@ -173,8 +195,8 @@
          if (value instanceof MarshalledValue) {
             value = ((MarshalledValue) value).get();
          }
-         InternalCacheEntry newEntry = Immutables.immutableInternalCacheEntry(InternalEntryFactory.create(key, value, 
-                  entry.getCreated(), entry.getLifespan(), entry.getLastUsed(), entry.getMaxIdle()));
+         InternalCacheEntry newEntry = Immutables.immutableInternalCacheEntry(InternalEntryFactory.create(key, value,
+                 entry.getCreated(), entry.getLifespan(), entry.getLastUsed(), entry.getMaxIdle()));
          copy.add(newEntry);
       }
       return Immutables.immutableSetWrap(copy);
@@ -183,15 +205,15 @@
    @Override
    public Object visitReplaceCommand(InvocationContext ctx, ReplaceCommand command) throws Throwable {
       MarshalledValue key = null, newValue = null, oldValue = null;
-      if (!MarshalledValue.isTypeExcluded(command.getKey().getClass())) {
+      if (!isTypeExcluded(command.getKey().getClass())) {
          key = createMarshalledValue(command.getKey(), ctx);
          command.setKey(key);
       }
-      if (!MarshalledValue.isTypeExcluded(command.getNewValue().getClass())) {
+      if (!isTypeExcluded(command.getNewValue().getClass())) {
          newValue = createMarshalledValue(command.getNewValue(), ctx);
          command.setNewValue(newValue);
       }
-      if (command.getOldValue() != null && !MarshalledValue.isTypeExcluded(command.getOldValue().getClass())) {
+      if (command.getOldValue() != null && !isTypeExcluded(command.getOldValue().getClass())) {
          oldValue = createMarshalledValue(command.getOldValue(), ctx);
          command.setOldValue(oldValue);
       }
@@ -203,7 +225,7 @@
    }
 
    private Object compactAndProcessRetVal(Set<MarshalledValue> marshalledValues, Object retVal)
-         throws IOException, ClassNotFoundException {
+           throws IOException, ClassNotFoundException {
       if (trace) log.trace("Compacting MarshalledValues created");
       for (MarshalledValue mv : marshalledValues) compact(mv);
       return processRetVal(retVal);
@@ -233,8 +255,8 @@
       for (Map.Entry me : m.entrySet()) {
          Object key = me.getKey();
          Object value = me.getValue();
-         Object newKey = (key == null || MarshalledValue.isTypeExcluded(key.getClass())) ? key : createMarshalledValue(key, ctx);
-         Object newValue = (value == null || MarshalledValue.isTypeExcluded(value.getClass())) ? value : createMarshalledValue(value, ctx);
+         Object newKey = (key == null || isTypeExcluded(key.getClass())) ? key : createMarshalledValue(key, ctx);
+         Object newValue = (value == null || isTypeExcluded(value.getClass())) ? value : createMarshalledValue(value, ctx);
          if (newKey instanceof MarshalledValue) marshalledValues.add((MarshalledValue) newKey);
          if (newValue instanceof MarshalledValue) marshalledValues.add((MarshalledValue) newValue);
          copy.put(newKey, newValue);

Modified: branches/4.1.x/core/src/main/java/org/infinispan/util/concurrent/locks/LockManager.java
===================================================================
--- branches/4.1.x/core/src/main/java/org/infinispan/util/concurrent/locks/LockManager.java	2010-08-04 17:15:42 UTC (rev 2164)
+++ branches/4.1.x/core/src/main/java/org/infinispan/util/concurrent/locks/LockManager.java	2010-08-04 17:21:25 UTC (rev 2165)
@@ -109,4 +109,10 @@
     * Cleanups the locks within the given context.
     */
    void releaseLocks(InvocationContext ctx);
+
+   /**
+    * Retrieves the number of locks currently held.
+    * @return an integer
+    */
+   int getNumberOfLocksHeld();
 }

Modified: branches/4.1.x/core/src/test/java/org/infinispan/context/MarshalledValueContextTest.java
===================================================================
--- branches/4.1.x/core/src/test/java/org/infinispan/context/MarshalledValueContextTest.java	2010-08-04 17:15:42 UTC (rev 2164)
+++ branches/4.1.x/core/src/test/java/org/infinispan/context/MarshalledValueContextTest.java	2010-08-04 17:21:25 UTC (rev 2165)
@@ -12,6 +12,8 @@
 import org.infinispan.test.TestingUtil;
 import org.infinispan.test.fwk.TestCacheManagerFactory;
 import org.infinispan.transaction.lookup.DummyTransactionManagerLookup;
+import org.infinispan.util.concurrent.locks.LockManager;
+import org.infinispan.util.concurrent.locks.containers.LockContainer;
 import org.testng.annotations.Test;
 
 import javax.transaction.TransactionManager;
@@ -45,19 +47,23 @@
       c.getAdvancedCache().lock(new Key("k"));
 
       InvocationContextContainer icc = TestingUtil.extractComponent(c, InvocationContextContainer.class);
+
+      LockManager lockManager = TestingUtil.extractComponent(c, LockManager.class);
       InvocationContext ctx = icc.getInvocationContext();
 
       assert ctx instanceof LocalTxInvocationContext;
 
       assert ctx.getLookedUpEntries().size() == 1 : "Looked up key should now be in the transactional invocation context";
+      assert lockManager.getNumberOfLocksHeld() == 1 : "Only one lock should be held";
 
       c.put(new Key("k"), "v2");
 
       assert ctx.getLookedUpEntries().size() == 1 : "Still should only be one entry in the context";
+      assert lockManager.getNumberOfLocksHeld() == 1 : "Only one lock should be held";
 
       tm.commit();
 
-      assert ctx.getLookedUpEntries().size() == 0 : "Context should be cleared of looked up keys";
+      assert lockManager.getNumberOfLocksHeld() == 0 : "No locks should be held anymore";
 
       assert "v2".equals(c.get(new Key("k")));
    }



More information about the infinispan-commits mailing list