Skip to content

Commit 2c0e87a

Browse files
cac03mp911de
authored andcommitted
DATAREDIS-1034 - Fix visibility issues in RedisTemplate.
Previously it was possible in multi-threaded environment to: * get a `null` from `opsFor*` methods in RedisTemplate. * fail with a `NullPointerException` when working with a not-`null` reference returned from `RedisTemplate.opsFor*` methods. Introduced changes initialize the fields eagerly to avoid visibility issues when accessing them and add a `final` modifier to the `AbstractOperations#template` field. Original pull request: #479.
1 parent 5838a10 commit 2c0e87a

File tree

2 files changed

+11
-33
lines changed

2 files changed

+11
-33
lines changed

src/main/java/org/springframework/data/redis/core/AbstractOperations.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
* @author Christoph Strobl
4545
* @author David Liu
4646
* @author Mark Paluch
47+
* @author Denis Zavedeev
4748
*/
4849
abstract class AbstractOperations<K, V> {
4950

@@ -64,7 +65,7 @@ public final V doInRedis(RedisConnection connection) {
6465
protected abstract byte[] inRedis(byte[] rawKey, RedisConnection connection);
6566
}
6667

67-
RedisTemplate<K, V> template;
68+
final RedisTemplate<K, V> template;
6869

6970
AbstractOperations(RedisTemplate<K, V> template) {
7071
this.template = template;

src/main/java/org/springframework/data/redis/core/RedisTemplate.java

+9-32
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@
7979
* @author Ninad Divadkar
8080
* @author Anqing Shao
8181
* @author Mark Paluch
82+
* @author Denis Zavedeev
8283
* @param <K> the Redis key type against which the template works (usually a String)
8384
* @param <V> the Redis value type against which the template works
8485
* @see StringRedisTemplate
@@ -100,13 +101,13 @@ public class RedisTemplate<K, V> extends RedisAccessor implements RedisOperation
100101

101102
private @Nullable ScriptExecutor<K> scriptExecutor;
102103

103-
// cache singleton objects (where possible)
104-
private @Nullable ValueOperations<K, V> valueOps;
105-
private @Nullable ListOperations<K, V> listOps;
106-
private @Nullable SetOperations<K, V> setOps;
107-
private @Nullable ZSetOperations<K, V> zSetOps;
108-
private @Nullable GeoOperations<K, V> geoOps;
109-
private @Nullable HyperLogLogOperations<K, V> hllOps;
104+
private final ValueOperations<K, V> valueOps = new DefaultValueOperations<>(this);
105+
private final ListOperations<K, V> listOps = new DefaultListOperations<>(this);
106+
private final SetOperations<K, V> setOps = new DefaultSetOperations<>(this);
107+
private final ZSetOperations<K, V> zSetOps = new DefaultZSetOperations<>(this);
108+
private final GeoOperations<K, V> geoOps = new DefaultGeoOperations<>(this);
109+
private final HyperLogLogOperations<K, V> hllOps = new DefaultHyperLogLogOperations<>(this);
110+
private final ClusterOperations<K, V> clusterOps = new DefaultClusterOperations<>(this);
110111

111112
/**
112113
* Constructs a new <code>RedisTemplate</code> instance.
@@ -1200,7 +1201,7 @@ public void slaveOfNoOne() {
12001201
*/
12011202
@Override
12021203
public ClusterOperations<K, V> opsForCluster() {
1203-
return new DefaultClusterOperations<>(this);
1204+
return clusterOps;
12041205
}
12051206

12061207
/*
@@ -1209,10 +1210,6 @@ public ClusterOperations<K, V> opsForCluster() {
12091210
*/
12101211
@Override
12111212
public GeoOperations<K, V> opsForGeo() {
1212-
1213-
if (geoOps == null) {
1214-
geoOps = new DefaultGeoOperations<>(this);
1215-
}
12161213
return geoOps;
12171214
}
12181215

@@ -1249,10 +1246,6 @@ public <HK, HV> HashOperations<K, HK, HV> opsForHash() {
12491246
*/
12501247
@Override
12511248
public HyperLogLogOperations<K, V> opsForHyperLogLog() {
1252-
1253-
if (hllOps == null) {
1254-
hllOps = new DefaultHyperLogLogOperations<>(this);
1255-
}
12561249
return hllOps;
12571250
}
12581251

@@ -1262,10 +1255,6 @@ public HyperLogLogOperations<K, V> opsForHyperLogLog() {
12621255
*/
12631256
@Override
12641257
public ListOperations<K, V> opsForList() {
1265-
1266-
if (listOps == null) {
1267-
listOps = new DefaultListOperations<>(this);
1268-
}
12691258
return listOps;
12701259
}
12711260

@@ -1293,10 +1282,6 @@ public BoundSetOperations<K, V> boundSetOps(K key) {
12931282
*/
12941283
@Override
12951284
public SetOperations<K, V> opsForSet() {
1296-
1297-
if (setOps == null) {
1298-
setOps = new DefaultSetOperations<>(this);
1299-
}
13001285
return setOps;
13011286
}
13021287

@@ -1315,10 +1300,6 @@ public BoundValueOperations<K, V> boundValueOps(K key) {
13151300
*/
13161301
@Override
13171302
public ValueOperations<K, V> opsForValue() {
1318-
1319-
if (valueOps == null) {
1320-
valueOps = new DefaultValueOperations<>(this);
1321-
}
13221303
return valueOps;
13231304
}
13241305

@@ -1337,10 +1318,6 @@ public BoundZSetOperations<K, V> boundZSetOps(K key) {
13371318
*/
13381319
@Override
13391320
public ZSetOperations<K, V> opsForZSet() {
1340-
1341-
if (zSetOps == null) {
1342-
zSetOps = new DefaultZSetOperations<>(this);
1343-
}
13441321
return zSetOps;
13451322
}
13461323

0 commit comments

Comments
 (0)