Skip to content

Commit 4fee380

Browse files
committed
HBASE-26582 Prune use of Random and SecureRandom objects
Avoid the pattern where a Random object is allocated, used once or twice, and then left for GC. This pattern triggers warnings from some static analysis tools because this pattern leads to poor effective randomness. In a few cases we were legitimately suffering from this issue; in others a change is still good to reduce noise in analysis results. Use ThreadLocalRandom where there is no requirement to set the seed to gain good reuse. Where useful relax use of SecureRandom to simply Random or ThreadLocalRandom, which are unlikely to block if the system entropy pool is low, if we don't need crypographically strong randomness for the use case. The exception to this is normalization of use of Bytes#random to fill byte arrays with randomness. Because Bytes#random may be used to generate key material it must be backed by SecureRandom.
1 parent a49c758 commit 4fee380

File tree

145 files changed

+483
-579
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

145 files changed

+483
-579
lines changed

hbase-asyncfs/src/test/java/org/apache/hadoop/hbase/io/asyncfs/TestFanOutOneBlockAsyncDFSOutput.java

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,11 @@ public class TestFanOutOneBlockAsyncDFSOutput extends AsyncFSTestBase {
7272
HBaseClassTestRule.forClass(TestFanOutOneBlockAsyncDFSOutput.class);
7373

7474
private static final Logger LOG = LoggerFactory.getLogger(TestFanOutOneBlockAsyncDFSOutput.class);
75-
7675
private static DistributedFileSystem FS;
77-
7876
private static EventLoopGroup EVENT_LOOP_GROUP;
79-
8077
private static Class<? extends Channel> CHANNEL_CLASS;
81-
8278
private static int READ_TIMEOUT_MS = 2000;
79+
private static final Random RNG = new Random();
8380

8481
private static StreamSlowMonitor MONITOR;
8582

@@ -108,10 +105,10 @@ static void writeAndVerify(FileSystem fs, Path f, AsyncFSOutput out)
108105
throws IOException, InterruptedException, ExecutionException {
109106
List<CompletableFuture<Long>> futures = new ArrayList<>();
110107
byte[] b = new byte[10];
111-
Random rand = new Random(12345);
112108
// test pipelined flush
109+
RNG.setSeed(12345);
113110
for (int i = 0; i < 10; i++) {
114-
rand.nextBytes(b);
111+
RNG.nextBytes(b);
115112
out.write(b);
116113
futures.add(out.flush(false));
117114
futures.add(out.flush(false));
@@ -123,11 +120,11 @@ static void writeAndVerify(FileSystem fs, Path f, AsyncFSOutput out)
123120
out.close();
124121
assertEquals(b.length * 10, fs.getFileStatus(f).getLen());
125122
byte[] actual = new byte[b.length];
126-
rand.setSeed(12345);
123+
RNG.setSeed(12345);
127124
try (FSDataInputStream in = fs.open(f)) {
128125
for (int i = 0; i < 10; i++) {
129126
in.readFully(actual);
130-
rand.nextBytes(b);
127+
RNG.nextBytes(b);
131128
assertArrayEquals(b, actual);
132129
}
133130
assertEquals(-1, in.read());

hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRegionHDFSBlockLocationFinder.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,19 +63,19 @@ public class TestRegionHDFSBlockLocationFinder {
6363
public static final HBaseClassTestRule CLASS_RULE =
6464
HBaseClassTestRule.forClass(TestRegionHDFSBlockLocationFinder.class);
6565

66+
private static final Random RNG = new Random();
6667
private static TableDescriptor TD;
67-
6868
private static List<RegionInfo> REGIONS;
6969

7070
private RegionHDFSBlockLocationFinder finder;
7171

7272
private static HDFSBlocksDistribution generate(RegionInfo region) {
7373
HDFSBlocksDistribution distribution = new HDFSBlocksDistribution();
7474
int seed = region.hashCode();
75-
Random rand = new Random(seed);
76-
int size = 1 + rand.nextInt(10);
75+
RNG.setSeed(seed);
76+
int size = 1 + RNG.nextInt(10);
7777
for (int i = 0; i < size; i++) {
78-
distribution.addHostsAndBlockWeight(new String[] { "host-" + i }, 1 + rand.nextInt(100));
78+
distribution.addHostsAndBlockWeight(new String[] { "host-" + i }, 1 + RNG.nextInt(100));
7979
}
8080
return distribution;
8181
}

hbase-client/src/main/java/org/apache/hadoop/hbase/client/PerClientRandomNonceGenerator.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,8 @@
1919
package org.apache.hadoop.hbase.client;
2020

2121
import java.util.Arrays;
22-
import java.util.Random;
23-
2422
import org.apache.hadoop.hbase.HConstants;
23+
import org.apache.hbase.thirdparty.io.netty.util.internal.ThreadLocalRandom;
2524
import org.apache.yetus.audience.InterfaceAudience;
2625

2726
/**
@@ -33,12 +32,12 @@ public final class PerClientRandomNonceGenerator implements NonceGenerator {
3332

3433
private static final PerClientRandomNonceGenerator INST = new PerClientRandomNonceGenerator();
3534

36-
private final Random rdm = new Random();
3735
private final long clientId;
3836

3937
private PerClientRandomNonceGenerator() {
4038
byte[] clientIdBase = ClientIdGenerator.generateClientId();
41-
this.clientId = (((long) Arrays.hashCode(clientIdBase)) << 32) + rdm.nextInt();
39+
this.clientId = (((long) Arrays.hashCode(clientIdBase)) << 32) +
40+
ThreadLocalRandom.current().nextInt();
4241
}
4342

4443
@Override
@@ -50,7 +49,7 @@ public long getNonceGroup() {
5049
public long newNonce() {
5150
long result = HConstants.NO_NONCE;
5251
do {
53-
result = rdm.nextLong();
52+
result = ThreadLocalRandom.current().nextLong();
5453
} while (result == HConstants.NO_NONCE);
5554
return result;
5655
}

hbase-client/src/main/java/org/apache/hadoop/hbase/filter/RandomRowFilter.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
package org.apache.hadoop.hbase.filter;
2121

2222
import java.util.Objects;
23-
import java.util.Random;
23+
import java.util.concurrent.ThreadLocalRandom;
2424

2525
import org.apache.hadoop.hbase.Cell;
2626
import org.apache.yetus.audience.InterfaceAudience;
@@ -35,7 +35,6 @@
3535
*/
3636
@InterfaceAudience.Public
3737
public class RandomRowFilter extends FilterBase {
38-
protected static final Random random = new Random();
3938

4039
protected float chance;
4140
protected boolean filterOutRow;
@@ -98,7 +97,7 @@ public boolean filterRowKey(Cell firstRowCell) {
9897
filterOutRow = false;
9998
} else {
10099
// roll the dice
101-
filterOutRow = !(random.nextFloat() < chance);
100+
filterOutRow = !(ThreadLocalRandom.current().nextFloat() < chance);
102101
}
103102
return filterOutRow;
104103
}

hbase-client/src/main/java/org/apache/hadoop/hbase/slowlog/SlowLogTableAccessor.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@
2222
import java.io.IOException;
2323
import java.util.ArrayList;
2424
import java.util.List;
25-
import java.util.Random;
25+
import java.util.concurrent.ThreadLocalRandom;
26+
2627
import org.apache.hadoop.conf.Configuration;
2728
import org.apache.hadoop.hbase.HConstants;
2829
import org.apache.hadoop.hbase.NamespaceDescriptor;
@@ -49,8 +50,6 @@ public class SlowLogTableAccessor {
4950

5051
private static final Logger LOG = LoggerFactory.getLogger(SlowLogTableAccessor.class);
5152

52-
private static final Random RANDOM = new Random();
53-
5453
private static Connection connection;
5554

5655
/**
@@ -139,7 +138,7 @@ private static byte[] getRowKey(final TooSlowLog.SlowLogPayload slowLogPayload)
139138
String lastFiveDig =
140139
hashcode.substring((hashcode.length() > 5) ? (hashcode.length() - 5) : 0);
141140
if (lastFiveDig.startsWith("-")) {
142-
lastFiveDig = String.valueOf(RANDOM.nextInt(99999));
141+
lastFiveDig = String.valueOf(ThreadLocalRandom.current().nextInt(99999));
143142
}
144143
final long currentTime = EnvironmentEdgeManager.currentTime();
145144
final String timeAndHashcode = currentTime + lastFiveDig;

hbase-client/src/test/java/org/apache/hadoop/hbase/security/TestEncryptionUtil.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323

2424
import java.security.Key;
2525
import java.security.KeyException;
26-
import java.security.SecureRandom;
26+
2727
import javax.crypto.spec.SecretKeySpec;
2828
import org.apache.hadoop.conf.Configuration;
2929
import org.apache.hadoop.hbase.HBaseClassTestRule;
@@ -110,7 +110,7 @@ public void testWALKeyWrappingWithIncorrectKey() throws Exception {
110110

111111
// generate a test key
112112
byte[] keyBytes = new byte[AES.KEY_LENGTH];
113-
new SecureRandom().nextBytes(keyBytes);
113+
Bytes.random(keyBytes);
114114
String algorithm = conf.get(HConstants.CRYPTO_WAL_ALGORITHM_CONF_KEY, HConstants.CIPHER_AES);
115115
Key key = new SecretKeySpec(keyBytes, algorithm);
116116

@@ -152,7 +152,7 @@ private void testKeyWrapping(String hashAlgorithm) throws Exception {
152152

153153
// generate a test key
154154
byte[] keyBytes = new byte[AES.KEY_LENGTH];
155-
new SecureRandom().nextBytes(keyBytes);
155+
Bytes.random(keyBytes);
156156
String algorithm =
157157
conf.get(HConstants.CRYPTO_KEY_ALGORITHM_CONF_KEY, HConstants.CIPHER_AES);
158158
Key key = new SecretKeySpec(keyBytes, algorithm);
@@ -189,7 +189,7 @@ private void testWALKeyWrapping(String hashAlgorithm) throws Exception {
189189

190190
// generate a test key
191191
byte[] keyBytes = new byte[AES.KEY_LENGTH];
192-
new SecureRandom().nextBytes(keyBytes);
192+
Bytes.random(keyBytes);
193193
String algorithm = conf.get(HConstants.CRYPTO_WAL_ALGORITHM_CONF_KEY, HConstants.CIPHER_AES);
194194
Key key = new SecretKeySpec(keyBytes, algorithm);
195195

@@ -214,7 +214,7 @@ private void testKeyWrappingWithMismatchingAlgorithms(Configuration conf) throws
214214

215215
// generate a test key
216216
byte[] keyBytes = new byte[AES.KEY_LENGTH];
217-
new SecureRandom().nextBytes(keyBytes);
217+
Bytes.random(keyBytes);
218218
String algorithm =
219219
conf.get(HConstants.CRYPTO_KEY_ALGORITHM_CONF_KEY, HConstants.CIPHER_AES);
220220
Key key = new SecretKeySpec(keyBytes, algorithm);

hbase-client/src/test/java/org/apache/hadoop/hbase/util/TestRoundRobinPoolMap.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,9 @@
2525
import java.util.Collections;
2626
import java.util.Iterator;
2727
import java.util.List;
28-
import java.util.Random;
2928
import java.util.concurrent.CompletableFuture;
3029
import java.util.concurrent.CompletionException;
3130
import java.util.concurrent.ExecutionException;
32-
import java.util.concurrent.ThreadLocalRandom;
3331
import java.util.concurrent.atomic.AtomicInteger;
3432
import org.apache.hadoop.hbase.HBaseClassTestRule;
3533
import org.apache.hadoop.hbase.testclassification.MiscTests;

hbase-client/src/test/java/org/apache/hadoop/hbase/util/TestThreadLocalPoolMap.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,9 @@
2020
import static org.junit.Assert.assertEquals;
2121

2222
import java.io.IOException;
23-
import java.util.Random;
2423
import java.util.concurrent.CompletableFuture;
2524
import java.util.concurrent.CompletionException;
2625
import java.util.concurrent.ExecutionException;
27-
import java.util.concurrent.ThreadLocalRandom;
2826
import java.util.concurrent.atomic.AtomicInteger;
2927
import org.apache.hadoop.hbase.HBaseClassTestRule;
3028
import org.apache.hadoop.hbase.testclassification.MiscTests;

hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/HFileBlockDefaultEncodingContext.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import java.io.DataOutputStream;
2222
import java.io.IOException;
2323
import java.io.InputStream;
24-
import java.security.SecureRandom;
2524

2625
import org.apache.hadoop.conf.Configuration;
2726
import org.apache.hadoop.hbase.io.ByteArrayOutputStream;
@@ -110,7 +109,7 @@ public HFileBlockDefaultEncodingContext(Configuration conf, DataBlockEncoding en
110109
if (cryptoContext != Encryption.Context.NONE) {
111110
cryptoByteStream = new ByteArrayOutputStream();
112111
iv = new byte[cryptoContext.getCipher().getIvLength()];
113-
new SecureRandom().nextBytes(iv);
112+
Bytes.random(iv);
114113
}
115114

116115
dummyHeader = Preconditions.checkNotNull(headerBytes,

hbase-common/src/main/java/org/apache/hadoop/hbase/util/Bytes.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2358,6 +2358,8 @@ public static void zero(byte[] b, int offset, int length) {
23582358
Arrays.fill(b, offset, offset + length, (byte) 0);
23592359
}
23602360

2361+
// SecureRandom is more expensive than other options but Bytes.random may be used to create
2362+
// key material.
23612363
private static final SecureRandom RNG = new SecureRandom();
23622364

23632365
/**

0 commit comments

Comments
 (0)