Skip to content

Commit 64acdd2

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 75806cc commit 64acdd2

File tree

2 files changed

+12
-38
lines changed

2 files changed

+12
-38
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

+10-37
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
* @author Ninad Divadkar
8282
* @author Anqing Shao
8383
* @author Mark Paluch
84+
* @author Denis Zavedeev
8485
* @param <K> the Redis key type against which the template works (usually a String)
8586
* @param <V> the Redis value type against which the template works
8687
* @see StringRedisTemplate
@@ -102,14 +103,14 @@ public class RedisTemplate<K, V> extends RedisAccessor implements RedisOperation
102103

103104
private @Nullable ScriptExecutor<K> scriptExecutor;
104105

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

114115
/**
115116
* Constructs a new <code>RedisTemplate</code> instance.
@@ -1203,7 +1204,7 @@ public void slaveOfNoOne() {
12031204
*/
12041205
@Override
12051206
public ClusterOperations<K, V> opsForCluster() {
1206-
return new DefaultClusterOperations<>(this);
1207+
return clusterOps;
12071208
}
12081209

12091210
/*
@@ -1212,10 +1213,6 @@ public ClusterOperations<K, V> opsForCluster() {
12121213
*/
12131214
@Override
12141215
public GeoOperations<K, V> opsForGeo() {
1215-
1216-
if (geoOps == null) {
1217-
geoOps = new DefaultGeoOperations<>(this);
1218-
}
12191216
return geoOps;
12201217
}
12211218

@@ -1252,10 +1249,6 @@ public <HK, HV> HashOperations<K, HK, HV> opsForHash() {
12521249
*/
12531250
@Override
12541251
public HyperLogLogOperations<K, V> opsForHyperLogLog() {
1255-
1256-
if (hllOps == null) {
1257-
hllOps = new DefaultHyperLogLogOperations<>(this);
1258-
}
12591252
return hllOps;
12601253
}
12611254

@@ -1265,10 +1258,6 @@ public HyperLogLogOperations<K, V> opsForHyperLogLog() {
12651258
*/
12661259
@Override
12671260
public ListOperations<K, V> opsForList() {
1268-
1269-
if (listOps == null) {
1270-
listOps = new DefaultListOperations<>(this);
1271-
}
12721261
return listOps;
12731262
}
12741263

@@ -1296,10 +1285,6 @@ public BoundSetOperations<K, V> boundSetOps(K key) {
12961285
*/
12971286
@Override
12981287
public SetOperations<K, V> opsForSet() {
1299-
1300-
if (setOps == null) {
1301-
setOps = new DefaultSetOperations<>(this);
1302-
}
13031288
return setOps;
13041289
}
13051290

@@ -1309,10 +1294,6 @@ public SetOperations<K, V> opsForSet() {
13091294
*/
13101295
@Override
13111296
public <HK, HV> StreamOperations<K, HK, HV> opsForStream() {
1312-
1313-
if (streamOps == null) {
1314-
streamOps = new DefaultStreamOperations<>(this, new ObjectHashMapper());
1315-
}
13161297
return (StreamOperations<K, HK, HV>) streamOps;
13171298
}
13181299

@@ -1350,10 +1331,6 @@ public BoundValueOperations<K, V> boundValueOps(K key) {
13501331
*/
13511332
@Override
13521333
public ValueOperations<K, V> opsForValue() {
1353-
1354-
if (valueOps == null) {
1355-
valueOps = new DefaultValueOperations<>(this);
1356-
}
13571334
return valueOps;
13581335
}
13591336

@@ -1372,10 +1349,6 @@ public BoundZSetOperations<K, V> boundZSetOps(K key) {
13721349
*/
13731350
@Override
13741351
public ZSetOperations<K, V> opsForZSet() {
1375-
1376-
if (zSetOps == null) {
1377-
zSetOps = new DefaultZSetOperations<>(this);
1378-
}
13791352
return zSetOps;
13801353
}
13811354

0 commit comments

Comments
 (0)