Code Review Request for 7107613, 7107616, 7185471
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