Code Review Request for 7107613, 7107616, 7185471 (original) (raw)

Valerie (Yu-Ching) Peng valerie.peng at oracle.com
Thu Jul 26 18:46:01 UTC 2012

Xuelei,

I pondered with the "preserving the Vector 'perms' and add a new transient ConcurrentHashMap 'permsMap'" idea in one of my previous changes. Felt that it's cleaner to only have one assuming no lengthy/complicated custom serialization methods needed. I will look at the ObjectOutputStream.* aspect some more to see how to properly address this.

Thanks for the good catch! Valerie

On 07/26/12 05:54, Xuelei Fan wrote:

On 7/26/2012 8:21 AM, Valerie (Yu-Ching) Peng wrote:

Hi, Max or Xuelei,

Either of you have cycles to review the following 3 perf related fixes? 7107613: scalability bloker in javax.crypto.CryptoPermissions 7107616: scalability bloker in javax.crypto.JceSecurityManager 7185471: Avoid key expansion when AES cipher is re-init w/ the same key Webrevs are at: http://cr.openjdk.java.net/~valeriep/7107613 In this update, the filed "perms" was declared as transient: ---------- - private Hashtable<String, PermissionCollection> perms; + private transient ConcurrentHashMap<String,PermissionCollection> perms; ---------- The ObjectOutputStream.PutField and ObjectInputStream.GetField will call: ---------- + Hashtable<String,PermissionCollection> permTable = + (Hashtable<String,PermissionCollection>) (fields.get("perms", null)); + Hashtable<String,PermissionCollection> permTable = + (Hashtable<String,PermissionCollection>) (fields.get("perms", null)); ---------- As will try to get the field offset of "perms" from the instance of this class, CryptoPermissions. Because "perms" has been declared as transient, a exception is properly thrown, java.lang.IllegalArgumentException: no such field perms with type class java.lang.Object I think we may need to override the "serialPersistentFields" of ObjectStreamField, as [1][2]: + private static final ObjectStreamField[] serialPersistentFields = { + new ObjectStreamField("perms", Hashtable.class), + }; + + private void readObject(ObjectInputStream s) + throws IOException, ClassNotFoundException { + ...

Alternatively, I was wondering maybe we reserver the hashtable variable, "perms", and add a new transient ConcurrentHashMap variable, "transientPerms": ------------------ private Hashtable<String, PermissionCollection> perms; + private transient + ConcurrentHashMap<String,PermissionCollection> transientPerms; ------------------ Then we will not need to override the serialPersistentFields variable any more. The readObject and writeObject looks like: ------------------ + private void readObject(ObjectInputStream s) + throws IOException, ClassNotFoundException { + + s.defaultReadObject(); + transientPerms = new ConcurrentHashMap<>(perms); + perms = null; + } + + private void writeObject(ObjectOutputStream s) throws IOException { + perms = new Hashtable<>(transientPerms); + s.defaultWriteObject(); + perms = null; + } ------------------ Xuelei [1] http://docs.oracle.com/javase/6/docs/platform/serialization/spec/input.html#4936 [2] http://docs.oracle.com/javase/6/docs/platform/serialization/spec/class.html#3127 http://cr.openjdk.java.net/~valeriep/7107616 http://cr.openjdk.java.net/~valeriep/7185471 The changes are for JDK 8. May be backported to 7u later if necessary, Thanks, Valerie

More information about the security-dev mailing list