0

Before the world of CompletableFuture, if I wanted to lock a variable, I could do this:

  private ReentrantLock myLock = new ReentrantLock();
  private List<UUID> myLockableList = new ArrayList<>();

  public void doStuff()
  {
    try
    {
      myLock.lock();
      myLockableList.clear();
    }
    finally
    {
      if (myLock.isLocked())
      {
        myLock.unlock();
      }
    }
  }

(Obviously, this is a very simplified example, I have other methods trying to operate on myLockableList, too).

According to ReentrantLock, 1 of 3 scenarios would occur:

  • Acquires the lock if it is not held by another thread and returns immediately, setting the lock hold count to one
  • If the current thread already holds the lock then the hold count is incremented by one and the method returns immediately
  • If the lock is held by another thread then the current thread becomes disabled for thread scheduling purposes and lies dormant until the lock has been acquired, at which time the lock hold count is set to one.

That's all great and exactly how I expect it to behave: if working in the same thread, I should know whether I have locked the resource, and if another thread has locked the resource, I would like to wait until it becomes available.

Enter CompletableFutures...

  private ReentrantLock myLock = new ReentrantLock();
  private List<UUID> myLockableList = new ArrayList<>();

  public CompletionStage<Void> doStuff()
  {
    return CompletableFuture.runAsync(() -> {
      try
      {
        myLock.lock();
        myLockableList.clear();
      }
      finally
      {
        if (myLock.isLocked())
        {
          myLock.unlock();
        }
      }
    });
  }

I would hope the behaviour for CompletableFuture would be the same. However, A CompletableFuture might execute the above with a new thread, or (if my understanding is correct) it might use a thread that is already in use. If the second happens (the thread being reused has already acquired the lock), then my lock would not pause the thread and wait for the lock, instead, it might actually return immediately (looking like it's acquired the lock)!

I can't seem to find a definitive answer in the documentation. Have I understood the situation correctly? If so, how should we be locking resources when using CompletableFuture?

Many thanks for any advice!

danwag
  • 355
  • 2
  • 14
  • 2
    "It might use a thread that is already in use". No it might not, because the thread is in use. It may use a free thread, which will try to acquire the lock as normal. Your `finally` clause is wrong, you're not supposed to check for `isLocked()`. You `lock()` in the beginning of the `try` and you `unlock()` in the `finally` clause. No ifs. – Kayaman Jan 14 '20 at 11:37
  • OK, thank you. So my understanding was not correct :) and so I have no problem with locking. – danwag Jan 14 '20 at 11:41
  • 1
    Well, you do (with the `isLocked()`), so you might want to go through this https://docs.oracle.com/javase/tutorial/essential/concurrency/index.html – Kayaman Jan 14 '20 at 11:43
  • I will have a look - thanks, W.R.T `isLocked()`, I had a method with some stuff before the `lock()`, which, depending on the result, might mean `lock()` is not called. When this happened, calling `unlock()` threw and Illegal exception. I could have used a nested try-finally block, but that was getting messy – danwag Jan 14 '20 at 11:45
  • but yes, `isLocked()` doesn't mean it was locked by the calling thread! Good point! Looks like I'll have to put back my nested try-finally – danwag Jan 14 '20 at 11:47
  • 2
    The usual (although very rare) use case for `isLocked()` is with `tryLock()`. Even better is to use the higher level concurrency primitives so you don't need to deal with raw locks. I recommend reading the concurrency tutorial. – Kayaman Jan 14 '20 at 11:49
  • Thanks for the link - looks like some reading is in order! – danwag Jan 14 '20 at 11:51
  • `isLocked()` could also mean that it was locked by the locker, not by you. So you would unlock it too soon. – Didier L Jan 14 '20 at 16:06

1 Answers1

1

In addition to the concurrency tutorial recommended in the comments, the Javadoc for ReentrantLock has great info that's worth reading.

This, for example:

A ReentrantLock is owned by the thread last successfully locking, but not yet unlocking it.

Your example uses isLocked(), which Javadoc says you shouldn't do:

This method is designed for use in monitoring of the system state, not for synchronization control.

Finally, the Javadoc includes a great usage example showing lock() in a "try" block followed by unlock() in the "finally" block.

It is recommended practice to always immediately follow a call to lock with a try block, most typically in a before/after construction such as:

 class X {
   private final ReentrantLock lock = new ReentrantLock();
   // ...

   public void m() {
     lock.lock();  // block until condition holds
     try {
       // ... method body
     } finally {
       lock.unlock()
     }
   }
 }
Kaan
  • 5,434
  • 3
  • 19
  • 41
  • I agree with everything you said - I put the `isLocked()` method in without looking it up first. My real misunderstanding came from thinking CompletableFutures reused paused threads. – danwag Jan 14 '20 at 18:28