harmony-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mark Hindess (JIRA)" <j...@apache.org>
Subject [jira] Reopened: (HARMONY-6661) Synchonrize on mutable field in Permissions.java
Date Wed, 29 Sep 2010 16:57:33 GMT

     [ https://issues.apache.org/jira/browse/HARMONY-6661?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Mark Hindess reopened HARMONY-6661:
-----------------------------------


There are some other readObject methods with sync blocks perhaps we should fix them before
closing this?  See:

  bash$ find classlib -name '*.java' -print0|xargs -0 perl -ne 'if (/^(\s+)private void readObject\(/)
{ $end = "^$1}\s*\$"; } elsif (/$end/) { undef $end } if (defined $end && /synchronized/)
{ print "$ARGV $.: $_\n";} } continue { close ARGV if eof' 
classlib/modules/beans/src/main/java/java/beans/beancontext/BeanContextServicesSupport.java
line 1073:         synchronized (bcsListeners) {

classlib/modules/beans/src/main/java/java/beans/beancontext/BeanContextSupport.java line 1298:
        synchronized (bcmListeners) {

classlib/modules/security/src/main/java/common/java/security/UnresolvedPermissionCollection.java
line 177:         synchronized (klasses) {

classlib/modules/security/src/main/java/common/java/security/BasicPermissionCollection.java
line 187:         synchronized (this) {

classlib/modules/security/src/main/java/common/java/security/Permissions.java line 217:  
      synchronized (klasses) {

(In case anyone is wondering about the '} continue {' in the perl line, it is to get $. reset
on eof so the line numbers are correct.)

> Synchonrize on mutable field in Permissions.java
> ------------------------------------------------
>
>                 Key: HARMONY-6661
>                 URL: https://issues.apache.org/jira/browse/HARMONY-6661
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>    Affects Versions: 6.0M1
>         Environment: Windows XP
>            Reporter: Wendy Feng
>            Assignee: Regis Xu
>             Fix For: 5.0M16
>
>   Original Estimate: 5h
>  Remaining Estimate: 5h
>
> 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);
>             }
>         }
>     ...
>   }
> ...
> }

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message