Skip to content

Commit

Permalink
ISPN-5443 NPE in ReadCommittedEntry.getLifespan() during Cache.size()
Browse files Browse the repository at this point in the history
* Fixed issue with RR when calling size when entry was not found in same
* transaction
  • Loading branch information
wburns authored and pruivo committed May 7, 2015
1 parent bade76b commit ddd9633
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 5 deletions.
Expand Up @@ -48,7 +48,7 @@ public Integer perform(InvocationContext ctx) throws Throwable {
CacheEntry value = contextEntries.get(entry.getKey());
if (value != null) {
keys.add(entry.getKey());
if (!value.isRemoved()) {
if (!value.isRemoved() && !value.isNull()) {
if (size++ == Integer.MAX_VALUE) {return Integer.MAX_VALUE;}
}
} else {
Expand All @@ -59,7 +59,7 @@ public Integer perform(InvocationContext ctx) throws Throwable {

// We can only add context entries if we didn't see it in iterator and it isn't removed
for (Map.Entry<Object, CacheEntry> entry : contextEntries.entrySet()) {
if (!keys.contains(entry.getKey()) && !entry.getValue().isRemoved()) {
if (!keys.contains(entry.getKey()) && !entry.getValue().isRemoved() && !entry.getValue().isNull()) {
if (size++ == Integer.MAX_VALUE) { return Integer.MAX_VALUE; }
}
}
Expand Down
Expand Up @@ -79,7 +79,7 @@ protected CacheEntry<K, C> getNextFromIterator() {
while (returnedEntry == null && !contextEntries.isEmpty() &&
(entry = contextEntries.remove(0)) != null) {
seenContextKeys.add(entry.getKey());
if (!ctx.isEntryRemovedInContext(entry.getKey())) {
if (!ctx.isEntryRemovedInContext(entry.getKey()) && !entry.isNull()) {
returnedEntry = filterEntry(entry);
}
}
Expand All @@ -89,7 +89,7 @@ protected CacheEntry<K, C> getNextFromIterator() {
CacheEntry contextEntry;
// If the value was in the context then we ignore the stored value since we use the context value
if ((contextEntry = ctx.lookupEntry(iteratedEntry.getKey())) != null) {
if (seenContextKeys.add(contextEntry.getKey()) && !contextEntry.isRemoved() &&
if (seenContextKeys.add(contextEntry.getKey()) && !contextEntry.isRemoved() && !contextEntry.isNull() &&
(returnedEntry = filterEntry(contextEntry)) != null) {
break;
}
Expand All @@ -104,7 +104,7 @@ protected CacheEntry<K, C> getNextFromIterator() {
if (returnedEntry == null) {
// We do a last check to make sure no additional values were added to our context while iterating
for (CacheEntry lookedUpEntry : ctx.getLookedUpEntries().values()) {
if (seenContextKeys.add(lookedUpEntry.getKey()) && !lookedUpEntry.isRemoved()) {
if (seenContextKeys.add(lookedUpEntry.getKey()) && !lookedUpEntry.isRemoved() && !lookedUpEntry.isNull()) {
if (returnedEntry == null) {
returnedEntry = lookedUpEntry;
} else {
Expand Down
26 changes: 26 additions & 0 deletions core/src/test/java/org/infinispan/api/APIRepeatableReadTxTest.java
@@ -0,0 +1,26 @@
package org.infinispan.api;

import org.infinispan.configuration.cache.ConfigurationBuilder;
import org.infinispan.manager.EmbeddedCacheManager;
import org.infinispan.test.fwk.TestCacheManagerFactory;
import org.infinispan.util.concurrent.IsolationLevel;
import org.testng.annotations.Test;

/**
* @author William Burns
* @since 7.2
*/
@Test (groups = "functional", testName = "api.APIRepeatableReadTxTest")
public class APIRepeatableReadTxTest extends APITxTest {

@Override
protected EmbeddedCacheManager createCacheManager() throws Exception {
// start a single cache instance
ConfigurationBuilder c = getDefaultStandaloneCacheConfig(true);
c.locking().isolationLevel(IsolationLevel.REPEATABLE_READ);
EmbeddedCacheManager cm = TestCacheManagerFactory.createCacheManager(false);
cm.defineConfiguration("test", c.build());
cache = cm.getCache("test");
return cm;
}
}
106 changes: 106 additions & 0 deletions core/src/test/java/org/infinispan/api/APITxTest.java
@@ -0,0 +1,106 @@
package org.infinispan.api;

import org.infinispan.configuration.cache.ConfigurationBuilder;
import org.infinispan.manager.EmbeddedCacheManager;
import org.infinispan.test.TestingUtil;
import org.infinispan.test.fwk.TestCacheManagerFactory;
import org.testng.annotations.Test;

import javax.transaction.NotSupportedException;
import javax.transaction.SystemException;
import javax.transaction.TransactionManager;

import static org.testng.AssertJUnit.assertEquals;

/**
* @author William Burns
* @since 7.2
*/
@Test (groups = "functional", testName = "api.APITxTest")
public class APITxTest extends APINonTxTest {

@Override
protected EmbeddedCacheManager createCacheManager() throws Exception {
// start a single cache instance
ConfigurationBuilder c = getDefaultStandaloneCacheConfig(true);
EmbeddedCacheManager cm = TestCacheManagerFactory.createCacheManager(false);
cm.defineConfiguration("test", c.build());
cache = cm.getCache("test");
return cm;
}

public void testSizeInExplicitTxWithNonExistent() throws SystemException, NotSupportedException {
assertEquals(0, cache.size());
cache.put("k", "v");

TransactionManager tm1 = TestingUtil.getTransactionManager(cache);
tm1.begin();
try {
cache.get("no-exist");
assertEquals(1, cache.size());
cache.put("no-exist", "value");
assertEquals(2, cache.size());
} finally {
tm1.rollback();
}
}

public void testSizeInExplicitTxWithRemoveNonExistent() throws SystemException, NotSupportedException {
assertEquals(0, cache.size());
cache.put("k", "v");

TransactionManager tm1 = TestingUtil.getTransactionManager(cache);
tm1.begin();
try {
cache.remove("no-exist");
assertEquals(1, cache.size());
cache.put("no-exist", "value");
assertEquals(2, cache.size());
} finally {
tm1.rollback();
}
}

public void testSizeInExplicitTxWithRemoveExistent() throws SystemException, NotSupportedException {
assertEquals(0, cache.size());
cache.put("k", "v");

TransactionManager tm1 = TestingUtil.getTransactionManager(cache);
tm1.begin();
try {
cache.put("exist", "value");
assertEquals(2, cache.size());
cache.remove("exist");
assertEquals(1, cache.size());
} finally {
tm1.rollback();
}
}

public void testSizeInExplicitTx() throws SystemException, NotSupportedException {
assertEquals(0, cache.size());
cache.put("k", "v");

TransactionManager tm1 = TestingUtil.getTransactionManager(cache);
tm1.begin();
try {
assertEquals(1, cache.size());
} finally {
tm1.rollback();
}
}

public void testSizeInExplicitTxWithModification() throws SystemException, NotSupportedException {
assertEquals(0, cache.size());
cache.put("k1", "v1");

TransactionManager tm1 = TestingUtil.getTransactionManager(cache);
tm1.begin();
try {
cache.put("k2", "v2");
assertEquals(2, cache.size());
} finally {
tm1.rollback();
}
}
}

0 comments on commit ddd9633

Please sign in to comment.