Repository: ignite Updated Branches: refs/heads/ignite-5227 [created] 0faa693ae
IGNITE-5227 - Fixed StackOverflowError in checkOwnerChanged callback Project: http://git-wip-us.apache.org/repos/asf/ignite/repo Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/0faa693a Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/0faa693a Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/0faa693a Branch: refs/heads/ignite-5227 Commit: 0faa693ae63d5a8bb8138ba83c3adfb6cc888516 Parents: abb9d94 Author: Alexey Goncharuk <[email protected]> Authored: Wed May 31 18:48:59 2017 +0300 Committer: Alexey Goncharuk <[email protected]> Committed: Wed May 31 18:48:59 2017 +0300 ---------------------------------------------------------------------- .../processors/cache/GridCacheMapEntry.java | 15 ++- .../distributed/GridDistributedCacheEntry.java | 25 +++-- .../cache/local/GridLocalCacheEntry.java | 26 +++-- .../cache/CacheLockCandidatesThreadTest.java | 107 +++++++++++++++++++ .../testsuites/IgniteCacheTestSuite5.java | 3 + 5 files changed, 163 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ignite/blob/0faa693a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheMapEntry.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheMapEntry.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheMapEntry.java index 7c7fc99..51b8799 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheMapEntry.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheMapEntry.java @@ -3682,6 +3682,18 @@ public abstract class GridCacheMapEntry extends GridMetadataAwareAdapter impleme protected final void checkOwnerChanged(@Nullable CacheLockCandidates prevOwners, @Nullable CacheLockCandidates owners, CacheObject val) { + checkOwnerChanged(prevOwners, owners, val, null); + } + /** + * @param prevOwners Previous owners. + * @param owners Current owners. + * @param val Entry value. + * @param checkCandidateChain flag to enable or disable check of candidate chain + */ + protected final void checkOwnerChanged(@Nullable CacheLockCandidates prevOwners, + @Nullable CacheLockCandidates owners, + CacheObject val, + CacheLockCandidates checkingCandidate) { assert !Thread.holdsLock(this); if (prevOwners != null && owners == null) { @@ -3717,7 +3729,8 @@ public abstract class GridCacheMapEntry extends GridMetadataAwareAdapter impleme if (locked) { cctx.mvcc().callback().onOwnerChanged(this, owner); - if (owner.local()) + if (owner.local() + && (checkingCandidate == null || !checkingCandidate.hasCandidate(owner.version()))) checkThreadChain(owner); if (cctx.events().isRecordable(EVT_CACHE_OBJECT_LOCKED)) { http://git-wip-us.apache.org/repos/asf/ignite/blob/0faa693a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/GridDistributedCacheEntry.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/GridDistributedCacheEntry.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/GridDistributedCacheEntry.java index e7675b7..dab9615 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/GridDistributedCacheEntry.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/GridDistributedCacheEntry.java @@ -589,7 +589,7 @@ public class GridDistributedCacheEntry extends GridCacheMapEntry { /** * Rechecks if lock should be reassigned. */ - public void recheck() { + public CacheLockCandidates recheck(GridCacheMvccCandidate checkingCandidate) { CacheLockCandidates prev = null; CacheLockCandidates owner = null; @@ -617,7 +617,9 @@ public class GridDistributedCacheEntry extends GridCacheMapEntry { } // This call must be made outside of synchronization. - checkOwnerChanged(prev, owner, val); + checkOwnerChanged(prev, owner, val, checkingCandidate); + + return owner; } /** {@inheritDoc} */ @@ -689,6 +691,8 @@ public class GridDistributedCacheEntry extends GridCacheMapEntry { assert owner.owner() || owner.used() : "Neither owner or used flags are set on ready local candidate: " + owner; + GridCacheMvccCandidate prev = owner; + if (owner.local() && owner.next() != null) { for (GridCacheMvccCandidate cand = owner.next(); cand != null; cand = cand.next()) { assert cand.local() : "Remote candidate cannot be part of thread chain: " + cand; @@ -700,11 +704,20 @@ public class GridDistributedCacheEntry extends GridCacheMapEntry { GridDistributedCacheEntry e = (GridDistributedCacheEntry)cctx0.cache().peekEx(cand.parent().key()); - if (e != null) - e.recheck(); - - break; + if (e != null) { + CacheLockCandidates newOnwer = e.recheck(owner); + + if(newOnwer == null || !newOnwer.hasCandidate(cand.version())) + // the lock from the chain hasn't been acquired, no sense to check the rest of the chain + break; + else + prev.next(null); + } + else + break; } + + prev = cand; } } } http://git-wip-us.apache.org/repos/asf/ignite/blob/0faa693a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/local/GridLocalCacheEntry.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/local/GridLocalCacheEntry.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/local/GridLocalCacheEntry.java index 421b32a..028cf54 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/local/GridLocalCacheEntry.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/local/GridLocalCacheEntry.java @@ -189,8 +189,9 @@ public class GridLocalCacheEntry extends GridCacheMapEntry { /** * Rechecks if lock should be reassigned. + * @return owner */ - public void recheck() { + public CacheLockCandidates recheck(GridCacheMvccCandidate checkingCandidate) { CacheObject val; CacheLockCandidates prev = null; CacheLockCandidates owner = null; @@ -210,7 +211,9 @@ public class GridLocalCacheEntry extends GridCacheMapEntry { val = this.val; } - checkOwnerChanged(prev, owner, val); + checkOwnerChanged(prev, owner, val, checkingCandidate); + + return owner; } /** {@inheritDoc} */ @@ -221,6 +224,8 @@ public class GridLocalCacheEntry extends GridCacheMapEntry { assert owner.owner() || owner.used() : "Neither owner or used flags are set on ready local candidate: " + owner; + GridCacheMvccCandidate prev = owner; + if (owner.next() != null) { for (GridCacheMvccCandidate cand = owner.next(); cand != null; cand = cand.next()) { assert cand.local(); @@ -233,11 +238,20 @@ public class GridLocalCacheEntry extends GridCacheMapEntry { // At this point candidate may have been removed and entry destroyed, // so we check for null. - if (e != null) - e.recheck(); - - break; + if (e != null) { + CacheLockCandidates newOwner = e.recheck(owner); + + if(newOwner == null || !newOwner.hasCandidate(cand.version())) + // the lock from the chain hasn't been acquired, no sense to check the rest of the chain + break; + else + prev.next(null); + } + else + break; } + + prev = cand; } } } http://git-wip-us.apache.org/repos/asf/ignite/blob/0faa693a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/CacheLockCandidatesThreadTest.java ---------------------------------------------------------------------- diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/CacheLockCandidatesThreadTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/CacheLockCandidatesThreadTest.java new file mode 100644 index 0000000..93b4b33 --- /dev/null +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/CacheLockCandidatesThreadTest.java @@ -0,0 +1,107 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.cache; + +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.concurrent.Callable; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.locks.Lock; +import org.apache.ignite.IgniteCache; +import org.apache.ignite.cache.CacheAtomicityMode; +import org.apache.ignite.cache.CacheMode; +import org.apache.ignite.configuration.CacheConfiguration; +import org.apache.ignite.internal.IgniteInternalFuture; +import org.apache.ignite.lang.IgniteFuture; +import org.apache.ignite.testframework.GridTestUtils; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; + +/** + * Tests locking of thread of candidates (see IGNITE-5227) + */ +public class CacheLockCandidatesThreadTest extends GridCommonAbstractTest { + /** */ + private static final String DEFAULT_CACHE_NAME = "default"; + + /** + * @throws Exception if failed. + */ + public void testLockCandidatesThreadForLocalMode() throws Exception { + lockThreadOfCandidates(CacheMode.LOCAL); + } + + /** {@inheritDoc} */ + @Override protected void beforeTestsStarted() throws Exception { + startGrid(0); + } + + /** {@inheritDoc} */ + @Override protected void afterTestsStopped() throws Exception { + stopAllGrids(); + } + + /** + * @throws Exception if failed. + */ + public void testLockCandidatesThreadForPartitionedMode() throws Exception { + lockThreadOfCandidates(CacheMode.PARTITIONED); + } + + private void lockThreadOfCandidates(CacheMode mode) throws Exception { + grid(0).createCache(new CacheConfiguration<Integer, Integer>(DEFAULT_CACHE_NAME) + .setAtomicityMode(CacheAtomicityMode.TRANSACTIONAL) + .setCacheMode(mode)); + + try { + final CountDownLatch releaseLatch = new CountDownLatch(1); + + final IgniteCache<Object, Object> cache = grid(0).cache(DEFAULT_CACHE_NAME); + IgniteInternalFuture<Object> future = GridTestUtils.runAsync(new Callable<Object>() { + @Override public Object call() throws Exception { + Lock lock = cache.lock("trigger"); + try { + lock.lock(); + releaseLatch.await(); + } finally { + lock.unlock(); + } + + return null; + } + }); + + Map<String, String> putMap = new LinkedHashMap<>(); + putMap.put("trigger", "trigger-new-val"); + for (int i = 0; i < 15_000; i++) + putMap.put("key-" + i, "value"); + + IgniteCache<Object, Object> asyncCache = grid(0).cache(DEFAULT_CACHE_NAME).withAsync(); + asyncCache.putAll(putMap); + IgniteFuture<Object> resFut = asyncCache.future(); + + Thread.sleep(500); + releaseLatch.countDown(); + + future.get(); + resFut.get(); + } + finally { + grid(0).destroyCache(DEFAULT_CACHE_NAME); + } + } +} http://git-wip-us.apache.org/repos/asf/ignite/blob/0faa693a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteCacheTestSuite5.java ---------------------------------------------------------------------- diff --git a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteCacheTestSuite5.java b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteCacheTestSuite5.java index 7baea2e..5f2b082 100644 --- a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteCacheTestSuite5.java +++ b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteCacheTestSuite5.java @@ -24,6 +24,7 @@ import org.apache.ignite.cache.affinity.AffinityClientNodeSelfTest; import org.apache.ignite.cache.affinity.AffinityHistoryCleanupTest; import org.apache.ignite.cache.affinity.local.LocalAffinityFunctionTest; import org.apache.ignite.internal.processors.cache.CacheKeepBinaryTransactionTest; +import org.apache.ignite.internal.processors.cache.CacheLockCandidatesThreadTest; import org.apache.ignite.internal.processors.cache.CacheNearReaderUpdateTest; import org.apache.ignite.internal.processors.cache.CacheRebalancingSelfTest; import org.apache.ignite.internal.processors.cache.CacheSerializableTransactionsTest; @@ -76,6 +77,8 @@ public class IgniteCacheTestSuite5 extends TestSuite { suite.addTestSuite(CacheRebalancingSelfTest.class); + suite.addTestSuite(CacheLockCandidatesThreadTest.class); + // Affinity tests. suite.addTestSuite(GridCacheAffinityBackupsSelfTest.class); suite.addTestSuite(IgniteCacheAffinitySelfTest.class);
