Should I use lock for reference assignment in class

58 Views Asked by At

I have a Spring component class with field of type list with fill from AWS DynamoDB. The use of this component class is in a controller for reading the list. I also have a schedule service that run in a cron and every 10 minute call the update list method.

The situation is that I will have multiple read threads and only one write thread, but I am not sure if I should use the Java concurrency lock. Also is the reference assignment atomic?

@Component
public class ExampleA  {
    DynamoDbEnhancedClient enhancedClient = DynamoDbEnhancedClient.builder()
            .dynamoDbClient(
                    // Configure an instance of the standard client.
                    DynamoDbClient.builder()
                            .region(Region.X)
                            .build())
            .build();

    DynamoDbTable<ClassFromDyanmo> table = enhancedClient.table("dynamo_table", TableSchema.fromBean(ClassFromDyanmo.class));
    private List<ClassFromDyanmo> listFromDynamo;
    
    public List<ClassFromDyanmo> getListFromDynamo() {
    return listFromDyanmo
}

public void updateListFromDynamo(){
    List<ClassFromDyanmo> result = new ArrayList<>();
    table.scan().forEach(page -> result.addAll(page.items()));
    listFromDyanmo = result;
}

the SchduleService:

@Service
public class UpdateRulesScheduler {

    @Autowired
    private ExampleA exampleA;

    @Scheduled(cron = "0 */10 * ? * *")
    public void scheduleEvery10Minutes() {
        exampleA.updateListFromDynamo();
    }
1

There are 1 best solutions below

2
rzwitserloot On

Yes, reference assignments are atomic and even guarantee consistent application, i.e. if you have this code:

// thread A:

someField = "Hello";
// do stuff
someField = "World";


// thread B:
print(someField);
// do stuff
print(someField);

Then it can print one of 3 things:

  • Hello / Hello
  • Hello / World
  • World / World

It is not possible for this code to asplode because of a half-written pointer, nor for this code to first see World and then later see Hello.

HOWEVER.

The problem is, this code is still highly suspect. Because it is quite likely it will print Hello/Hello even if (relative to computer terms, and in theory, even human terms, i.e. minutes or even days) have passed since the code that set it to "World" has ran! The JVM gives itself the right (see the Java Memory Model section of the JVM Specification) to create local caches for every field, so, thread B has a 'cache' that reads 'Hello', and when thread A updates that field to 'World', that just updates thread A's local cache of that field. A says that field has value 'World', B says it has value 'Hello', and that state of affairs can last very very long indeed, the JVM makes no guarantees about arbitrary synchronization of such conflicts.

What you need are so-called Happens-Before relationships in order to get around the fact that the JMM has all sorts of 'I may.. or may not' text in it (which doesn't simply mean this code is unreliable - it means it is untestable. On a really simple machine that doesn't have local CPU cache pages, it'll work just fine. On another it fails all the time - you can't figure that out when working on the box where it works fine!)

You establish this in many ways - there is an explicit list in the JMM section. The obvious ones are the same things that establish atomicity - synchronized, volatile, and many java core libs (mostly, some of stuff in the java.util.concurrent package. Not all the stuff - things like AtomicInteger don't, and that's why AtomicInteger is generally much faster than trying to handroll such things with synchronized, as synchronized does a lot more than just provide atomicity - it establishes guarantees).

The idea behind HB is simple: Any 2 statements have an HB relationship, or not. If they do, then it is not possible for the line that 'happens after' to observe any state as it was before the HB line ran. You shouldn't think about it in terms of 'the HB line runs first' (after all, if the HA line doesn't read any state that the HB line is changing, the JVM is not obligated to 'run the HB line first').

You will need to establish HB here: You want listFromDynamo = result; to be HB relative to whatever line has any interest in receiving this update in some sort of timely fashion. Otherwise the JVM is free to take days to propagate this write.

One really simple way to do it is to slap a synchronized on getListFromDynamo and updateListFromDynamo (or at least wrap listFromDynamo = result in a synchronized block). It's generally a really bad idea to lock on public stuff, so use lombok's @Synchronized or handwrite it:

private final Object dynamoLock = new Object[0];

public List<ClassFromDynamo> getListFromDynamo() {
  synchronized (dynamoLock) {
    return listFromDynamo;
  }
}

public void updateListFromDynamo(){
  List<ClassFromDyanmo> result = new ArrayList<>();
  table.scan().forEach(page -> result.addAll(page.items()));
  synchronized (dynamoLock) {
    listFromDyanmo = result;
  }
}

This establishes that the } of the synchronized block in the update method is HB relative to the synchronized (listFromDynamo) { in the get method, and therefore ensures that the get method cannot observe state as it was before - i.e. cannot observe the 'old' reference.

If that feels too 'inefficient' - perhaps look into that first, synchronized probably isn't nearly as bad as you think it is, especially in light of the fact that entire table scans are being performed here. That's like complaining that peeing in the ocean in Amsterdam raises the waterlevel in Manhattan. Yeah probably but not in a way you could even measure. If you have profiler reports that conclusively tell you there is indeed a performance issue there, then and only then start looking into alternatives, which do exist (AtomicReference might be very slightly faster, I'd try that first).