Uploaded image for project: 'yangtools'
  1. yangtools
  2. YANGTOOLS-241

Remove double-checked locking in ReadOnlyTrieMap

    XMLWordPrintable

Details

    • Bug
    • Status: Resolved
    • Resolution: Done
    • None
    • None
    • None
    • None
    • Operating System: All
      Platform: All

    • 1460
    • High

    Description

      ReadOnlyTrieMap uses the double-checked locking (DCL) technique in this method:

      protected Map<K, V> delegate() {
      if (readOnly == null) {
      synchronized (this) {
      if (readOnly == null)

      { readOnly = readWrite.readOnlySnapshot(); }

      }
      }

      return readOnly;
      }

      However DCL is not thread-safe. See http://www.javaworld.com/article/2074979/java-concurrency/double-checked-locking--clever--but-broken.html

      A few solutions:

      1) The simplest solution is to avoid lazy initialization and created readOnly in
      the ctor and make it final.

      2) If we really want lazy initialization the remove the DCL:

      protected Map<K, V> delegate() {
      synchronized (this) {
      if (readOnly == null)

      { readOnly = readWrite.readOnlySnapshot(); }

      }

      return readOnly;
      }

      The downside to synchronizing on 'this' is that users may also synchronize
      on the instance externally resulting in unwanted contention.

      3) Keep the lazy initialization and DCL but make readOnly volatile. However this
      would only be safe if readOnly is immutable (all fields final) or has all its
      fields volatile.

      DCL is usually not worth the trouble. Depending on usage, ie we can assume users will call delegate most of the time, I would opt for #1. Otherwise #2.

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          People

            rovarga Robert Varga
            tpantelis Tom Pantelis
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: