Uploaded image for project: 'Harmony'
  1. Harmony
  2. HARMONY-6661

Synchonrize on mutable field in Permissions.java

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 6.0M1
    • Fix Version/s: 5.0M16
    • Component/s: Classlib
    • Labels:
      None
    • Environment:
      Windows XP

      Description

      I found a unsafe synchronization in modules/security/src/main/java/common/java/security/Permissions.java
      public final class Permissions extends PermissionCollection implements Serializable {
      ...
      private void readObject(java.io.ObjectInputStream in) throws IOException,
      ClassNotFoundException {
      ...
      klasses = new HashMap();
      synchronized (klasses) {
      for (Iterator iter = perms.entrySet().iterator(); iter.hasNext() {
      Map.Entry entry = (Map.Entry) iter.next();
      Class key = (Class) entry.getKey();
      PermissionCollection pc = (PermissionCollection) entry.getValue();
      if (key != pc.elements().nextElement().getClass())

      { throw new InvalidObjectException(Messages.getString("security.22")); //$NON-NLS-1$ }
      klasses.put(key, pc);
      }
      }
      ...
      }
      ...
      }

      In the above code , a block is synchronized on klasses field. Before the synchronized block, klasses is assigned to a new value.

      Consequence:
      Different threads will synchronize on different klasses objects because it has been assigned to a new value. It breaks the mutual exclusion and update on klasses would be lost.

      I suggest to rewrite it as follow:
      public final class Permissions extends PermissionCollection implements Serializable {
      private static final Object monitor = new Object();
      ...
      private void readObject(java.io.ObjectInputStream in) throws IOException,
      ClassNotFoundException {
      ...
      klasses = new HashMap();
      synchronized (monitor ) {
      for (Iterator iter = perms.entrySet().iterator(); iter.hasNext() {
      Map.Entry entry = (Map.Entry) iter.next();
      Class key = (Class) entry.getKey();
      PermissionCollection pc = (PermissionCollection) entry.getValue();
      if (key != pc.elements().nextElement().getClass()) { throw new InvalidObjectException(Messages.getString("security.22")); //$NON-NLS-1$ }

      klasses.put(key, pc);
      }
      }
      ...
      }
      ...
      }

        Attachments

          Activity

            People

            • Assignee:
              chleicdl Ray Chen
              Reporter:
              cashcrop Wendy Feng
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 5h
                5h
                Remaining:
                Remaining Estimate - 5h
                5h
                Logged:
                Time Spent - Not Specified
                Not Specified