You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by ag...@apache.org on 2020/05/27 12:38:58 UTC
[ignite] branch master updated: IGNITE-13039 Get rid of possibility
to change final static fields - Fixes #7819.
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 <st...@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 <al...@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);
}
}