1
package com.blah;

import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import javax.annotation.PostConstruct;

import org.springframework.stereotype.Component;

import lombok.Value;

@Component
public class AdminCredentialsProvider {

    @Value
    public class AuthInfo {
        String authToken;
        String xsrfToken;
    }

    private AuthInfo adminAuthInfo = null;

    private final ScheduledExecutorService adminAuthInfoRefresher =
            Executors.newSingleThreadScheduledExecutor();

    //This will be called by multiple threads
    public AuthInfo getSiteAdminCredentials() {
        return new AuthInfo(adminAuthInfo.getAuthToken(), adminAuthInfo.getXsrfToken());
    }

    @PostConstruct
    public void init() {
        adminAuthInfo = loginAsSiteAdmin();

        Runnable runnable = () -> {
            //Do I need to protect this assignment with a lock at all
            adminAuthInfo = loginAsSiteAdmin();
        };

        adminAuthInfoRefresher.scheduleAtFixedRate(runnable, 1, 1, TimeUnit.HOURS);
    }

    private AuthInfo loginAsSiteAdmin() {
        //call rest endpoint to login
        return new AuthInfo("mockToken", "mockXsrf");
    }
}

adminAuthInfo variable is only set by one thread (after initial assignment).

The AuthInfo class is immutable

I am aware that the java = (reference assignment) is atomic.

I know I can implement this with AtomicReference, or ReadWriteLock. But is a lock necessary here at all?

arahant
  • 2,203
  • 7
  • 38
  • 62
  • 1
    No, not thread-safe. Other threads may not see the change made to the reference variable. In other words, the reference re-assignment may not be *visible* according to the rules of the [Java Memory Model](https://en.m.wikipedia.org/wiki/Java_memory_model). See [this Answer](https://stackoverflow.com/a/13032827/642706). Mandatory reading: http://www.JavaConcurrencyInPractice.com/ – Basil Bourque Oct 22 '21 at 02:19
  • 2
    Life will still be a lot simpler if you use `AtomicReference`, though. – chrylis -cautiouslyoptimistic- Oct 22 '21 at 02:24
  • Indeed, I agree with the other Comment. An [`AtomicReference`](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/concurrent/atomic/AtomicReference.html) is one solution, and is the solution I would use because it is simple and makes clear your intention. – Basil Bourque Oct 22 '21 at 02:26
  • @BasilBourque if I understand the other answer correctly, adding `volatile` would be enough here. I agree that using AtomicReference is just easier, and that's what I'll likely do. – arahant Oct 22 '21 at 02:54
  • Yes, using `volatile` is another solution. Given that the meaning of `volatile` has changed since the early years, and that many programmers do not understand the proper use of `volatile`, I would generally suggest the `Atomic…` classes over `volatile`. (Caveat: I am not an expert on concurrency.) – Basil Bourque Oct 22 '21 at 03:24

0 Answers0