Search code examples
javajava.util.concurrentdouble-checked-locking

How to remove double checking when setting a value in a multithreaded environment


I have a class Checker that stores the field archiveDate.There are several threads that can set this field. They only record the field if it is not yet available. And if one of the threads recorded the date, the other threads don't have to overwrite it. And there is another thread (the Checker itself) that can remove the date. It removes the date only if it is set. Now my code looks like this.

public class Checker extends Thread {
    private volatile LocalDateTime archiveDate;
    private ReentrantLock lock;

    public Checker() {
        lock = new ReentrantLock();
    }

    @Override
    public void run() {
        while (!Thread.currentThread().isInterrupted()) {
            if (archiveDate != null) {
                lock.lock();
                try {
                    if (archiveDate != null && archiveDate.isBefore(LocalDateTime.now())) {
                        //do something
                        archiveDate = null;
                    }
                } finally {
                    lock.unlock();
                }
            }
            try {
                TimeUnit.SECONDS.sleep(30);
            } catch (InterruptedException e) {
                Thread.currentThread().interrupt();
                break;
            }
        }
    }

    public void changeDate(LocalDateTime date) {
        if (archiveDate == null) {
            lock.lock();
            if (archiveDate == null) {
                try {
                    archiveDate = date;
                } finally {
                    lock.unlock();
                }
            }
        }
    }
}

Can I avoid double checks or simplify the code in some way?


Solution

  • It seems to me that you are using the wrong tool for the job. To execute a task at a particular datetime, use an adapter to a ScheduledExecutorService:

    static final ScheduledExecutorService ES = Executors.newSingleThreadScheduledExecutor();
    
    public static ScheduledFuture<?> schedule(LocalDateTime targetTime, Runnable r) {
        return ES.schedule(r,
            LocalDateTime.now().until(targetTime, ChronoUnit.NANOS), TimeUnit.NANOSECONDS);
    }
    

    To me, it looks a bit strange to ignore new requests when there is a pending one, regardless of the specified date, but a logic similar to your question’s code can be implemented using above schedule method as:

    class Checker implements Runnable {
        final AtomicReference<LocalDateTime> archiveDate = new AtomicReference<>();
    
        @Override
        public void run() {
            //do something
        
            archiveDate.set(null);
        }
    
        public void changeDate(LocalDateTime date) {
            if(archiveDate.compareAndSet(null, date)) {
                schedule(date, this);
            }
        }
    }