This is an automated email from the ASF dual-hosted git repository. agoncharuk 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 cbe58f4 IGNITE-13039 Get rid of possibility to change final static fields - Fixes #7819. cbe58f4 is described below commit cbe58f46ac05218f9ab7b7455a1dd32bba14b385 Author: zstan <stanilov...@gmail.com> AuthorDate: Wed May 27 15:36:22 2020 +0300 IGNITE-13039 Get rid of possibility to change final static fields - Fixes #7819. Signed-off-by: Alexey Goncharuk <alexey.goncha...@gmail.com> --- .../cache/transactions/TxDeadlockDetection.java | 8 ++-- .../TxDeadlockDetectionNoHangsTest.java | 12 +++--- .../apache/ignite/testframework/GridTestUtils.java | 45 +++++++++++----------- .../query/h2/twostep/AbstractReducer.java | 14 +++---- .../query/IgniteSqlSplitterSelfTest.java | 4 +- .../h2/twostep/RetryCauseMessageSelfTest.java | 28 +++++++------- 6 files changed, 55 insertions(+), 56 deletions(-) diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/TxDeadlockDetection.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/TxDeadlockDetection.java index a904e5b..576dec7 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/TxDeadlockDetection.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/TxDeadlockDetection.java @@ -52,7 +52,7 @@ import static org.apache.ignite.internal.processors.cache.transactions.IgniteTxM */ public class TxDeadlockDetection { /** Deadlock detection maximum iterations. */ - private static final int DEADLOCK_TIMEOUT = getInteger(IGNITE_TX_DEADLOCK_DETECTION_TIMEOUT, 60000); + private static int deadLockTimeout = getInteger(IGNITE_TX_DEADLOCK_DETECTION_TIMEOUT, 60000); /** Sequence. */ private static final AtomicLong SEQ = new AtomicLong(); @@ -229,7 +229,7 @@ public class TxDeadlockDetection { this.topVer = topVer; this.keys = keys; - if (DEADLOCK_TIMEOUT > 0) { + if (deadLockTimeout > 0) { timeoutObj = new DeadlockTimeoutObject(); cctx.time().addTimeoutObject(timeoutObj); @@ -555,7 +555,7 @@ public class TxDeadlockDetection { * Default constructor. */ DeadlockTimeoutObject() { - super(DEADLOCK_TIMEOUT); + super(deadLockTimeout); } /** {@inheritDoc} */ @@ -564,7 +564,7 @@ public class TxDeadlockDetection { IgniteLogger log = cctx.kernalContext().log(this.getClass()); - U.warn(log, "Deadlock detection was timed out [timeout=" + DEADLOCK_TIMEOUT + ", fut=" + this + ']'); + U.warn(log, "Deadlock detection was timed out [timeout=" + deadLockTimeout + ", fut=" + this + ']'); onDone(); } diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/TxDeadlockDetectionNoHangsTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/TxDeadlockDetectionNoHangsTest.java index 27a0799..7a7651b 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/TxDeadlockDetectionNoHangsTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/TxDeadlockDetectionNoHangsTest.java @@ -89,12 +89,12 @@ public class TxDeadlockDetectionNoHangsTest extends GridCommonAbstractTest { @Override protected void beforeTestsStarted() throws Exception { super.beforeTestsStarted(); - GridTestUtils.setFieldValue(null, TxDeadlockDetection.class, "DEADLOCK_TIMEOUT", (int)(getTestTimeout() * 2)); + GridTestUtils.setFieldValue(TxDeadlockDetection.class, "deadLockTimeout", (int)(getTestTimeout() * 2)); } /** {@inheritDoc} */ @Override protected void afterTestsStopped() throws Exception { - GridTestUtils.setFieldValue(null, TxDeadlockDetection.class, "DEADLOCK_TIMEOUT", + GridTestUtils.setFieldValue(TxDeadlockDetection.class, "deadLockTimeout", getInteger(IGNITE_TX_DEADLOCK_DETECTION_TIMEOUT, 60000)); } @@ -113,14 +113,14 @@ public class TxDeadlockDetectionNoHangsTest extends GridCommonAbstractTest { doTest(PESSIMISTIC); try { - GridTestUtils.setFieldValue(null, IgniteTxManager.class, "DEADLOCK_MAX_ITERS", 0); + GridTestUtils.setFieldValue(IgniteTxManager.class, "DEADLOCK_MAX_ITERS", 0); assertFalse(grid(0).context().cache().context().tm().deadlockDetectionEnabled()); doTest(PESSIMISTIC); } finally { - GridTestUtils.setFieldValue(null, IgniteTxManager.class, "DEADLOCK_MAX_ITERS", + GridTestUtils.setFieldValue(IgniteTxManager.class, "DEADLOCK_MAX_ITERS", IgniteSystemProperties.getInteger(IGNITE_TX_DEADLOCK_DETECTION_MAX_ITERS, 1000)); } } @@ -135,14 +135,14 @@ public class TxDeadlockDetectionNoHangsTest extends GridCommonAbstractTest { doTest(OPTIMISTIC); try { - GridTestUtils.setFieldValue(null, IgniteTxManager.class, "DEADLOCK_MAX_ITERS", 0); + GridTestUtils.setFieldValue(IgniteTxManager.class, "DEADLOCK_MAX_ITERS", 0); assertFalse(grid(0).context().cache().context().tm().deadlockDetectionEnabled()); doTest(OPTIMISTIC); } finally { - GridTestUtils.setFieldValue(null, IgniteTxManager.class, "DEADLOCK_MAX_ITERS", + GridTestUtils.setFieldValue(IgniteTxManager.class, "DEADLOCK_MAX_ITERS", IgniteSystemProperties.getInteger(IGNITE_TX_DEADLOCK_DETECTION_MAX_ITERS, 1000)); } } diff --git a/modules/core/src/test/java/org/apache/ignite/testframework/GridTestUtils.java b/modules/core/src/test/java/org/apache/ignite/testframework/GridTestUtils.java index f799322..608c52a 100644 --- a/modules/core/src/test/java/org/apache/ignite/testframework/GridTestUtils.java +++ b/modules/core/src/test/java/org/apache/ignite/testframework/GridTestUtils.java @@ -34,10 +34,8 @@ import java.net.InetAddress; import java.net.MulticastSocket; import java.net.ServerSocket; import java.nio.file.attribute.PosixFilePermission; -import java.security.AccessController; import java.security.GeneralSecurityException; import java.security.KeyStore; -import java.security.PrivilegedAction; import java.sql.Connection; import java.sql.DriverManager; import java.sql.SQLException; @@ -1622,27 +1620,6 @@ public final class GridTestUtils { } /** - * Change static final fields. - * @param field Need to be changed. - * @param newVal New value. - * @throws Exception If failed. - */ - public static void setFieldValue(Field field, Object newVal) throws Exception { - field.setAccessible(true); - Field modifiersField = Field.class.getDeclaredField("modifiers"); - - AccessController.doPrivileged(new PrivilegedAction() { - @Override public Object run() { - modifiersField.setAccessible(true); - return null; - } - }); - - modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL); - field.set(null, newVal); - } - - /** * Get inner class by its name from the enclosing class. * * @param parentCls Parent class to resolve inner class for. @@ -1674,6 +1651,18 @@ public final class GridTestUtils { Field field = cls.getDeclaredField(fieldName); + boolean isFinal = (field.getModifiers() & Modifier.FINAL) != 0; + + boolean isStatic = (field.getModifiers() & Modifier.STATIC) != 0; + + /** + * http://java.sun.com/docs/books/jls/third_edition/html/memory.html#17.5.3 + * If a final field is initialized to a compile-time constant in the field declaration, + * changes to the final field may not be observed. + */ + if (isFinal && isStatic) + throw new IgniteException("Modification of static final field through reflection."); + boolean accessible = field.isAccessible(); if (!accessible) @@ -1708,6 +1697,16 @@ public final class GridTestUtils { boolean isFinal = (field.getModifiers() & Modifier.FINAL) != 0; + boolean isStatic = (field.getModifiers() & Modifier.STATIC) != 0; + + /** + * http://java.sun.com/docs/books/jls/third_edition/html/memory.html#17.5.3 + * If a final field is initialized to a compile-time constant in the field declaration, + * changes to the final field may not be observed. + */ + if (isFinal && isStatic) + throw new IgniteException("Modification of static final field through reflection."); + if (isFinal) { Field modifiersField = Field.class.getDeclaredField("modifiers"); diff --git a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/AbstractReducer.java b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/AbstractReducer.java index dfb23d7..3b08fe1 100644 --- a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/AbstractReducer.java +++ b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/AbstractReducer.java @@ -54,16 +54,16 @@ public abstract class AbstractReducer implements Reducer { static final int MAX_FETCH_SIZE = getInteger(IGNITE_SQL_MERGE_TABLE_MAX_SIZE, 10_000); /** */ - static final int PREFETCH_SIZE = getInteger(IGNITE_SQL_MERGE_TABLE_PREFETCH_SIZE, 1024); + static int prefetchSize = getInteger(IGNITE_SQL_MERGE_TABLE_PREFETCH_SIZE, 1024); static { - if (!U.isPow2(PREFETCH_SIZE)) { - throw new IllegalArgumentException(IGNITE_SQL_MERGE_TABLE_PREFETCH_SIZE + " (" + PREFETCH_SIZE + + if (!U.isPow2(prefetchSize)) { + throw new IllegalArgumentException(IGNITE_SQL_MERGE_TABLE_PREFETCH_SIZE + " (" + prefetchSize + ") must be positive and a power of 2."); } - if (PREFETCH_SIZE >= MAX_FETCH_SIZE) { - throw new IllegalArgumentException(IGNITE_SQL_MERGE_TABLE_PREFETCH_SIZE + " (" + PREFETCH_SIZE + + if (prefetchSize >= MAX_FETCH_SIZE) { + throw new IllegalArgumentException(IGNITE_SQL_MERGE_TABLE_PREFETCH_SIZE + " (" + prefetchSize + ") must be less than " + IGNITE_SQL_MERGE_TABLE_MAX_SIZE + " (" + MAX_FETCH_SIZE + ")."); } } @@ -102,7 +102,7 @@ public abstract class AbstractReducer implements Reducer { AbstractReducer(GridKernalContext ctx) { this.ctx = ctx; - fetched = new ReduceBlockList<>(PREFETCH_SIZE); + fetched = new ReduceBlockList<>(prefetchSize); } /** {@inheritDoc} */ @@ -191,7 +191,7 @@ public abstract class AbstractReducer implements Reducer { * @param evictedBlock Evicted block. */ protected void onBlockEvict(@NotNull List<Row> evictedBlock) { - assert evictedBlock.size() == PREFETCH_SIZE; + assert evictedBlock.size() == prefetchSize; // Remember the last row (it will be max row) from the evicted block. lastEvictedRow = requireNonNull(last(evictedBlock)); diff --git a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteSqlSplitterSelfTest.java b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteSqlSplitterSelfTest.java index 92e0739..15dd2e1 100644 --- a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteSqlSplitterSelfTest.java +++ b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteSqlSplitterSelfTest.java @@ -567,7 +567,7 @@ public class IgniteSqlSplitterSelfTest extends AbstractIndexingCommonTest { Integer.class, Value.class)); try { - GridTestUtils.setFieldValue(null, AbstractReducer.class, "PREFETCH_SIZE", 8); + GridTestUtils.setFieldValue(AbstractReducer.class, "prefetchSize", 8); Random rnd = new GridRandom(); @@ -617,7 +617,7 @@ public class IgniteSqlSplitterSelfTest extends AbstractIndexingCommonTest { } } finally { - GridTestUtils.setFieldValue(null, AbstractReducer.class, "PREFETCH_SIZE", 1024); + GridTestUtils.setFieldValue(AbstractReducer.class, "prefetchSize", 1024); c.destroy(); } diff --git a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/twostep/RetryCauseMessageSelfTest.java b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/twostep/RetryCauseMessageSelfTest.java index 62f5f32..a1f670e 100644 --- a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/twostep/RetryCauseMessageSelfTest.java +++ b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/twostep/RetryCauseMessageSelfTest.java @@ -96,7 +96,7 @@ public class RetryCauseMessageSelfTest extends AbstractIndexingCommonTest { public void testSynthCacheWasNotFoundMessage() { GridMapQueryExecutor mapQryExec = GridTestUtils.getFieldValue(h2Idx, IgniteH2Indexing.class, "mapQryExec"); - GridTestUtils.setFieldValue(h2Idx, IgniteH2Indexing.class, "mapQryExec", + GridTestUtils.setFieldValue(h2Idx, "mapQryExec", new MockGridMapQueryExecutor() { @Override public void onQueryRequest(ClusterNode node, GridH2QueryRequest qryReq) throws IgniteCheckedException { @@ -121,7 +121,7 @@ public class RetryCauseMessageSelfTest extends AbstractIndexingCommonTest { return; } finally { - GridTestUtils.setFieldValue(h2Idx, IgniteH2Indexing.class, "mapQryExec", mapQryExec); + GridTestUtils.setFieldValue(h2Idx, "mapQryExec", mapQryExec); } fail(); } @@ -135,7 +135,7 @@ public class RetryCauseMessageSelfTest extends AbstractIndexingCommonTest { final ConcurrentMap<PartitionReservationKey, GridReservable> reservations = reservations(h2Idx); - GridTestUtils.setFieldValue(h2Idx, IgniteH2Indexing.class, "mapQryExec", + GridTestUtils.setFieldValue(h2Idx, "mapQryExec", new MockGridMapQueryExecutor() { @Override public void onQueryRequest(ClusterNode node, GridH2QueryRequest qryReq) throws IgniteCheckedException { @@ -166,7 +166,7 @@ public class RetryCauseMessageSelfTest extends AbstractIndexingCommonTest { return; } finally { - GridTestUtils.setFieldValue(h2Idx, IgniteH2Indexing.class, "mapQryExec", mapQryExec); + GridTestUtils.setFieldValue(h2Idx, "mapQryExec", mapQryExec); } fail(); } @@ -181,7 +181,7 @@ public class RetryCauseMessageSelfTest extends AbstractIndexingCommonTest { final GridKernalContext ctx = GridTestUtils.getFieldValue(mapQryExec, GridMapQueryExecutor.class, "ctx"); - GridTestUtils.setFieldValue(h2Idx, IgniteH2Indexing.class, "mapQryExec", + GridTestUtils.setFieldValue(h2Idx, "mapQryExec", new MockGridMapQueryExecutor() { @Override public void onQueryRequest(ClusterNode node, GridH2QueryRequest qryReq) throws IgniteCheckedException { GridCacheContext<?, ?> cctx = ctx.cache().context().cacheContext(qryReq.caches().get(0)); @@ -211,7 +211,7 @@ public class RetryCauseMessageSelfTest extends AbstractIndexingCommonTest { return; } finally { - GridTestUtils.setFieldValue(h2Idx, IgniteH2Indexing.class, "mapQryExec", mapQryExec); + GridTestUtils.setFieldValue(h2Idx, "mapQryExec", mapQryExec); } fail(); } @@ -225,7 +225,7 @@ public class RetryCauseMessageSelfTest extends AbstractIndexingCommonTest { final GridKernalContext ctx = GridTestUtils.getFieldValue(mapQryExec, GridMapQueryExecutor.class, "ctx"); - GridTestUtils.setFieldValue(h2Idx, IgniteH2Indexing.class, "mapQryExec", + GridTestUtils.setFieldValue(h2Idx, "mapQryExec", new MockGridMapQueryExecutor() { @Override public void onQueryRequest(ClusterNode node, GridH2QueryRequest qryReq) throws IgniteCheckedException { @@ -256,7 +256,7 @@ public class RetryCauseMessageSelfTest extends AbstractIndexingCommonTest { return; } finally { - GridTestUtils.setFieldValue(h2Idx, IgniteH2Indexing.class, "mapQryExec", mapQryExec); + GridTestUtils.setFieldValue(h2Idx, "mapQryExec", mapQryExec); } fail(); } @@ -270,7 +270,7 @@ public class RetryCauseMessageSelfTest extends AbstractIndexingCommonTest { final ConcurrentMap<PartitionReservationKey, GridReservable> reservations = reservations(h2Idx); - GridTestUtils.setFieldValue(h2Idx, IgniteH2Indexing.class, "mapQryExec", + GridTestUtils.setFieldValue(h2Idx, "mapQryExec", new MockGridMapQueryExecutor() { @Override public void onQueryRequest(ClusterNode node, GridH2QueryRequest qryReq) throws IgniteCheckedException { @@ -300,7 +300,7 @@ public class RetryCauseMessageSelfTest extends AbstractIndexingCommonTest { return; } finally { - GridTestUtils.setFieldValue(h2Idx, IgniteH2Indexing.class, "mapQryExec", mapQryExec); + GridTestUtils.setFieldValue(h2Idx, "mapQryExec", mapQryExec); } fail(); } @@ -316,7 +316,7 @@ public class RetryCauseMessageSelfTest extends AbstractIndexingCommonTest { final IgniteLogger logger = GridTestUtils.getFieldValue(rdcQryExec, GridReduceQueryExecutor.class, "log"); final GridKernalContext ctx = GridTestUtils.getFieldValue(rdcQryExec, GridReduceQueryExecutor.class, "ctx"); - GridTestUtils.setFieldValue(rdcQryExec, GridReduceQueryExecutor.class, "mapper", + GridTestUtils.setFieldValue(rdcQryExec, "mapper", new ReducePartitionMapper(ctx, logger) { @Override public ReducePartitionMapResult nodesForPartitions(List<Integer> cacheIds, AffinityTopologyVersion topVer, int[] parts, boolean isReplicatedOnly) { @@ -336,7 +336,7 @@ public class RetryCauseMessageSelfTest extends AbstractIndexingCommonTest { throwable.printStackTrace(); } finally { - GridTestUtils.setFieldValue(rdcQryExec, GridReduceQueryExecutor.class, "mapper", mapper); + GridTestUtils.setFieldValue(rdcQryExec, "mapper", mapper); } } @@ -351,7 +351,7 @@ public class RetryCauseMessageSelfTest extends AbstractIndexingCommonTest { final IgniteLogger logger = GridTestUtils.getFieldValue(rdcQryExec, GridReduceQueryExecutor.class, "log"); final GridKernalContext ctx = GridTestUtils.getFieldValue(rdcQryExec, GridReduceQueryExecutor.class, "ctx"); - GridTestUtils.setFieldValue(rdcQryExec, GridReduceQueryExecutor.class, "mapper", + GridTestUtils.setFieldValue(rdcQryExec, "mapper", new ReducePartitionMapper(ctx, logger) { @Override public ReducePartitionMapResult nodesForPartitions(List<Integer> cacheIds, AffinityTopologyVersion topVer, int[] parts, boolean isReplicatedOnly) { @@ -377,7 +377,7 @@ public class RetryCauseMessageSelfTest extends AbstractIndexingCommonTest { }, CacheException.class, "Failed to determine nodes participating in the update. "); } finally { - GridTestUtils.setFieldValue(rdcQryExec, GridReduceQueryExecutor.class, "mapper", mapper); + GridTestUtils.setFieldValue(rdcQryExec, "mapper", mapper); } }