This is an automated email from the ASF dual-hosted git repository. irakov pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/master by this push: new b6dbc9e IGNITE-5227 StackOverflowError in GridCacheMapEntry#checkOwnerChanged() - Fixes #6736. b6dbc9e is described below commit b6dbc9e781de9001c68007d9853938e85b217b87 Author: mstepachev <maksim.stepac...@gmail.com> AuthorDate: Thu Aug 1 19:47:08 2019 +0300 IGNITE-5227 StackOverflowError in GridCacheMapEntry#checkOwnerChanged() - Fixes #6736. Signed-off-by: Ivan Rakov <ira...@apache.org> --- .../processors/cache/GridCacheMapEntry.java | 16 ++- .../distributed/GridDistributedCacheEntry.java | 18 ++- .../cache/local/GridLocalCacheEntry.java | 19 ++- .../cache/CacheLockCandidatesThreadTest.java | 133 +++++++++++++++++++++ 4 files changed, 173 insertions(+), 13 deletions(-) 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 f1b7ec7..194c97a 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 @@ -4865,6 +4865,19 @@ 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 checkingCandidate flag to enable or disable check of candidate chain + */ + protected final void checkOwnerChanged(@Nullable CacheLockCandidates prevOwners, + @Nullable CacheLockCandidates owners, + CacheObject val, + CacheLockCandidates checkingCandidate) { assert !lock.isHeldByCurrentThread(); if (prevOwners != null && owners == null) { @@ -4900,7 +4913,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)) { 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 9b1d0ca..51245e8 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 @@ -642,7 +642,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; @@ -675,7 +675,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} */ @@ -753,10 +755,14 @@ 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 + break; } } } 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 e26174a..65f7c400 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 @@ -199,8 +199,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; @@ -225,7 +226,9 @@ public class GridLocalCacheEntry extends GridCacheMapEntry { unlockEntry(); } - checkOwnerChanged(prev, owner, val); + checkOwnerChanged(prev, owner, val, checkingCandidate); + + return owner; } /** {@inheritDoc} */ @@ -248,10 +251,14 @@ 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 + break; } } } 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..5062f94 --- /dev/null +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/CacheLockCandidatesThreadTest.java @@ -0,0 +1,133 @@ +/* + * 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 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; +import org.junit.Test; + +import java.util.Map; +import java.util.TreeMap; +import java.util.concurrent.Callable; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.locks.Lock; + +/** + * Tests locking of thread of candidates (see GG-17364) + */ +public class CacheLockCandidatesThreadTest extends GridCommonAbstractTest { + /** */ + private static final String DEFAULT_CACHE_NAME = "default"; + + /** + * @throws Exception if failed. + */ + @Test + public void testLockCandidatesThreadForLocalMode() throws Exception { + lockThreadOfCandidates(CacheMode.LOCAL); + } + + /** + * @throws Exception if failed. + */ + @Test + public void testLockCandidatesThreadForReplicatedMode() throws Exception { + lockThreadOfCandidates(CacheMode.REPLICATED); + } + + /** + * @throws Exception if failed. + */ + @Test + public void testLockCandidatesThreadForPartitionedMode() throws Exception { + lockThreadOfCandidates(CacheMode.PARTITIONED); + } + + /** + * @param mode Mode. + */ + private void lockThreadOfCandidates(CacheMode mode) throws Exception { + startGridsMultiThreaded(1); + + grid(0).createCache(new CacheConfiguration<Integer, Integer>(DEFAULT_CACHE_NAME) + .setAtomicityMode(CacheAtomicityMode.TRANSACTIONAL) + .setCacheMode(mode)); + + try { + final CountDownLatch unlock = new CountDownLatch(1); + final CountDownLatch locked = new CountDownLatch(1); + + final IgniteCache<Object, Object> cache = grid(0).cache(DEFAULT_CACHE_NAME); + + final String triggerKey = "" + ThreadLocalRandom.current().nextInt(); + + System.out.println("Trigger: " + triggerKey); + + cache.put(triggerKey, "val"); + + IgniteInternalFuture<Object> future = GridTestUtils.runAsync(new Callable<Object>() { + @Override public Object call() throws Exception { + Lock lock = cache.lock(triggerKey); + try { + lock.lock(); + + System.out.println("Trigger is locked"); + + locked.countDown(); + + unlock.await(); + } finally { + lock.unlock(); + + System.out.println("Trigger is unlocked"); + } + + return null; + } + }); + + locked.await(); + + Map<String, String> map = new TreeMap<>(); + + map.put(triggerKey, "trigger-new-val"); + + for (int i = 0; i < 4_000; i++) + map.put("key-" + i, "value"); + + IgniteFuture<Void> f = grid(0).cache(DEFAULT_CACHE_NAME).putAllAsync(map); + + Thread.sleep(200); + + unlock.countDown(); + + future.get(); + f.get(); + } + finally { + stopAllGrids(); + } + } +}