This is an automated email from the ASF dual-hosted git repository. ptupitsyn 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 c90a828 IGNITE-13128 Fix NPE when IgniteLock is removed before use c90a828 is described below commit c90a828490828eb2dac548d0ad29781df519ce0f Author: Kartik Somani <kartik.somani.jai...@gmail.com> AuthorDate: Mon Jun 15 13:52:42 2020 +0530 IGNITE-13128 Fix NPE when IgniteLock is removed before use When 2 IgniteLock objects are created, and one used only after the other one was closed, there was a NullPointerException due to missing initialization check. --- .../datastructures/GridCacheLockImpl.java | 3 ++ .../datastructures/IgniteLockAbstractSelfTest.java | 39 +++++++++++----------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/datastructures/GridCacheLockImpl.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/datastructures/GridCacheLockImpl.java index 04c533d..ff6d9e4 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/datastructures/GridCacheLockImpl.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/datastructures/GridCacheLockImpl.java @@ -1161,6 +1161,9 @@ public final class GridCacheLockImpl extends AtomicDataStructureProxy<GridCacheL try { initializeReentrantLock(); + if (sync == null) + throw new IgniteCheckedException("Failed to find reentrant lock with given name: " + name); + sync.lock(); sync.validate(false); diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/datastructures/IgniteLockAbstractSelfTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/datastructures/IgniteLockAbstractSelfTest.java index 5d3ed99..afe125b 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/datastructures/IgniteLockAbstractSelfTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/datastructures/IgniteLockAbstractSelfTest.java @@ -57,7 +57,6 @@ import org.apache.ignite.resources.LoggerResource; import org.apache.ignite.testframework.GridTestUtils; import org.apache.ignite.transactions.Transaction; import org.jetbrains.annotations.Nullable; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -1569,6 +1568,25 @@ public abstract class IgniteLockAbstractSelfTest extends IgniteAtomicsAbstractTe } /** + * Tests that closed lock throws meaningful exception. + */ + @Test (expected = IgniteException.class) + public void testClosedLockThrowsIgniteException() { + final String lockName = "lock"; + + IgniteLock lock1 = grid(0).reentrantLock(lockName, false, false, true); + IgniteLock lock2 = grid(0).reentrantLock(lockName, false, false, true); + lock1.close(); + try { + lock2.lock(); + } catch (IgniteException e) { + String msg = String.format("Failed to find reentrant lock with given name: " + lockName); + assertEquals(msg, e.getMessage()); + throw e; + } + } + + /** * Tests if lock is evenly acquired among nodes when fair flag is set on. * Since exact ordering of lock acquisitions cannot be guaranteed because it also depends * on the OS thread scheduling, certain deviation from uniform distribution is tolerated. @@ -1681,25 +1699,6 @@ public abstract class IgniteLockAbstractSelfTest extends IgniteAtomicsAbstractTe ignite.close(); } - /** - * Tests that closed lock throws meaningful exception. - */ - @Test - @Ignore("https://issues.apache.org/jira/browse/IGNITE-13128") - public void testClosedLockThrowsIgniteException() { - final String lockName = "testRemovedLockThrowsIgniteException"; - - Ignite srv = ignite(0); - - IgniteLock lock1 = srv.reentrantLock(lockName, false, false, true); - IgniteLock lock2 = srv.reentrantLock(lockName, false, false, true); - - lock1.close(); - - //noinspection ThrowableNotThrown - GridTestUtils.assertThrows(log, lock2::lock, IgniteException.class, "TODO"); - } - /** {@inheritDoc} */ @Override public void writeExternal(ObjectOutput out) throws IOException { // No-op.