From a739efc7a1bbba668bc8c20594479277797d78bf Mon Sep 17 00:00:00 2001
From: Sunjeet Singh <sunjeets@netflix.com>
Date: Sat, 7 Oct 2023 12:00:19 -0700
Subject: [PATCH] Read state concurrent access bugfix- atomicity in accessing
 fields of shard holder

---
 .../object/HollowObjectTypeReadState.java     | 31 +++++++++++++------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/object/HollowObjectTypeReadState.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/object/HollowObjectTypeReadState.java
index 9baa6b8501..3d667eb2cc 100644
--- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/object/HollowObjectTypeReadState.java
+++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/object/HollowObjectTypeReadState.java
@@ -360,63 +360,76 @@ public int readOrdinal(int ordinal, int fieldIndex) {
     @Override
     public int readInt(int ordinal, int fieldIndex) {
         sampler.recordFieldAccess(fieldIndex);
-        HollowObjectTypeReadStateShard shard = shardsVolatile.shards[ordinal & shardsVolatile.shardNumberMask];
+        ShardsHolder shardsHolder = shardsVolatile; // this read can return a stale shardHolder with greater or lesser than current
+                                                    // num shards but maxOrdinal remains same across stale vs current. So given this
+                                                    // atomic assignment below operations on a stale shards holder will be legal
+                                                    // SNAP: Here: but which shard to map to should probably be re-computed right, if
+                                                    // the we got a stale shard here?
+        HollowObjectTypeReadStateShard shard = shardsHolder.shards[ordinal & shardsHolder.shardNumberMask];
         return shard.readInt(ordinal >> shard.shardOrdinalShift, fieldIndex);
     }
 
     @Override
     public float readFloat(int ordinal, int fieldIndex) {
         sampler.recordFieldAccess(fieldIndex);
-        HollowObjectTypeReadStateShard shard = shardsVolatile.shards[ordinal & shardsVolatile.shardNumberMask];
+        ShardsHolder shardsHolder = shardsVolatile;
+        HollowObjectTypeReadStateShard shard = shardsHolder.shards[ordinal & shardsHolder.shardNumberMask];
         return shard.readFloat(ordinal >> shard.shardOrdinalShift, fieldIndex);
     }
 
     @Override
     public double readDouble(int ordinal, int fieldIndex) {
         sampler.recordFieldAccess(fieldIndex);
-        HollowObjectTypeReadStateShard shard = shardsVolatile.shards[ordinal & shardsVolatile.shardNumberMask];
+        ShardsHolder shardsHolder = shardsVolatile;
+        HollowObjectTypeReadStateShard shard = shardsHolder.shards[ordinal & shardsHolder.shardNumberMask];
         return shard.readDouble(ordinal >> shard.shardOrdinalShift, fieldIndex);
     }
 
     @Override
     public long readLong(int ordinal, int fieldIndex) {
         sampler.recordFieldAccess(fieldIndex);
-        HollowObjectTypeReadStateShard shard = shardsVolatile.shards[ordinal & shardsVolatile.shardNumberMask];
+        ShardsHolder shardsHolder = shardsVolatile;
+        HollowObjectTypeReadStateShard shard = shardsHolder.shards[ordinal & shardsHolder.shardNumberMask];
         return shard.readLong(ordinal >> shard.shardOrdinalShift, fieldIndex);
     }
 
     @Override
     public Boolean readBoolean(int ordinal, int fieldIndex) {
         sampler.recordFieldAccess(fieldIndex);
-        HollowObjectTypeReadStateShard shard = shardsVolatile.shards[ordinal & shardsVolatile.shardNumberMask];
+        ShardsHolder shardsHolder = shardsVolatile;
+        HollowObjectTypeReadStateShard shard = shardsHolder.shards[ordinal & shardsHolder.shardNumberMask];
         return shard.readBoolean(ordinal >> shard.shardOrdinalShift, fieldIndex);
     }
 
     @Override
     public byte[] readBytes(int ordinal, int fieldIndex) {
         sampler.recordFieldAccess(fieldIndex);
-        HollowObjectTypeReadStateShard shard = shardsVolatile.shards[ordinal & shardsVolatile.shardNumberMask];
+        ShardsHolder shardsHolder = shardsVolatile;
+        HollowObjectTypeReadStateShard shard = shardsHolder.shards[ordinal & shardsHolder.shardNumberMask];
         return shard.readBytes(ordinal >> shard.shardOrdinalShift, fieldIndex);
     }
 
     @Override
     public String readString(int ordinal, int fieldIndex) {
         sampler.recordFieldAccess(fieldIndex);
-        HollowObjectTypeReadStateShard shard = shardsVolatile.shards[ordinal & shardsVolatile.shardNumberMask];
+        ShardsHolder shardsHolder = shardsVolatile;
+        HollowObjectTypeReadStateShard shard = shardsHolder.shards[ordinal & shardsHolder.shardNumberMask];
         return shard.readString(ordinal >> shard.shardOrdinalShift, fieldIndex);
     }
 
     @Override
     public boolean isStringFieldEqual(int ordinal, int fieldIndex, String testValue) {
         sampler.recordFieldAccess(fieldIndex);
-        HollowObjectTypeReadStateShard shard = shardsVolatile.shards[ordinal & shardsVolatile.shardNumberMask];
+        ShardsHolder shardsHolder = shardsVolatile;
+        HollowObjectTypeReadStateShard shard = shardsHolder.shards[ordinal & shardsHolder.shardNumberMask];
         return shard.isStringFieldEqual(ordinal >> shard.shardOrdinalShift, fieldIndex, testValue);
     }
 
     @Override
     public int findVarLengthFieldHashCode(int ordinal, int fieldIndex) {
         sampler.recordFieldAccess(fieldIndex);
-        HollowObjectTypeReadStateShard shard = shardsVolatile.shards[ordinal & shardsVolatile.shardNumberMask];
+        ShardsHolder shardsHolder = shardsVolatile;
+        HollowObjectTypeReadStateShard shard = shardsHolder.shards[ordinal & shardsHolder.shardNumberMask];
         return shard.findVarLengthFieldHashCode(ordinal >> shard.shardOrdinalShift, fieldIndex);
     }