Uploaded image for project: 'netvirt'
  1. netvirt
  2. NETVIRT-39

Synchronization block on Mac String that is created in local scope

    XMLWordPrintable

Details

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

    • 6147

    Description

      In ElanPacketInHandler.onPacketReceived():
      byte[] srcMac = res.getSourceMACAddress();
      // You create a new String here
      String macAddress = NWUtil.toStringMacAddress(srcMac);

      // You call setupMacFlows with the new created String
      ElanUtils.setupMacFlows(…., macAddress)

      In ElanUtils.setupMacFlows():
      synchronized (macAddress) {
      ...
      }

      The macAddress String that the synchronized block is using as a mutex was created locally. If the same mac value will be used in a different place or by a different thread, it will have its own reference, thus not locking anything.

      A naive solution will be to use String.intern() but this is a very bad practice, as illustrated here: http://stackoverflow.com/questions/461896/what-is-the-most-frequent-concurrency-issue-youve-encountered-in-java/463437#463437.

      My suggested solution is to hold a map between macAddress to Object() and synchronized on the Object.
      Map will be created like this:
      CacheBuilder.newBuilder().weakValues()
      To obtain the lock, you will use:
      Object lock = Cache.asMap().putIfAbsent(mac, new Object());

      Because we are using weakValues(), there is no need to remove the entries from the map, as they will be garbage collected if there is no live reference to them.

      Attachments

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

        Activity

          People

            Unassigned Unassigned
            guy.sela@hpe.com Guy Sela
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: