Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8278255

Add more warning text in ReentrantLock and ReentrantReadWriteLock

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: P4 P4
    • 23
    • None
    • core-libs
    • None

      Before the example in the class specification of ReentrantLock, there is the text:

          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:

      This is reasonable as far as it goes, but it might be worthwhile to add some details. The call to `lock()` should occur *immediately before* the beginning of the try block (but not inside of it), with no intervening statements or expressions. The call to `unlock()` should occur as the *very first* statement of the finally block.

      The danger here is that somebody might put in an apparently innocuous statement (such as logging a message) that, if it were to throw an exception, would violate the locking invariants.

      The example in ReentrantReadWriteLock can lead to more insidious bugs. It's not incorrect, and the general recommendations still apply but more subtly. Specifically, the code is in a precarious state between the lock acquisition and the beginning of the try-finally statement that releases it. The code shouldn't do anything that throws any exceptions, lest the lock never be released. The example is:

          rwl.readLock().lock();
          if (!cacheValid) {
              ...
          }

          try {
              ...
          } finally {
              rwl.readLock().unlock();
          }

      In this code, initial read lock acquisition is quite far from the try-finally statement that protects it. It would be very easy for errors to creep in here. For example, an apparently innocuous refactoring from checking the boolean cacheValid field to calling an isCacheValid() method could introduce errors, if that method could possibly throw an exception.

      Naturally one could write volumes about this issue; that probably isn't appropriate for the class specifications in javadoc. However, some brief judicious warnings and exhortations highlighting the issue might be appropriate.

            vklang Viktor Klang
            smarks Stuart Marks
            Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

              Created:
              Updated:
              Resolved: