hg: jdk8/tl/jdk: 7103570: AtomicIntegerFieldUpdater does not work when SecurityManager is installed

David Holmes david.holmes at oracle.com
Tue May 8 10:22:24 UTC 2012


<restricting to core-libs-dev>

Hi Ulf,

On 8/05/2012 8:02 PM, Ulf Zibis wrote:
> Hi all,
>
> I'm a little bit late, but I just have seen:
> (1) some indentations in the patch are broken

<sigh> I missed a couple of indent-2 that should be indent-4.

And something went awry with indentation in ensureProtectedAccess. 
Unfortunately the webrev didn't show this and it isn't code that was 
functionally modified (I had a tab-to-space conversion issue, which is 
how this got touched at all).

> (2) following code snipped is repeated many times:

4 times.

> + ClassLoader cl = tclass.getClassLoader();
> + ClassLoader ccl = caller.getClassLoader();
> + if ((ccl != null) && (ccl != cl) &&
> + ((cl == null) || !isAncestor(cl, ccl))) {
> + sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
> + }
> Wouldn't it be better, to move it in a method, maybe in
> sun.reflect.misc.ReflectUtil ?

It didn't seem quite worth the effort of factoring out.

The isAncestor method is also repeated but again was not considered a 
worthwhile factorization.

Now that I'm writing that I'm having my doubts, but at the time it was 
expedient. Looking forward this code will likely have to change again to 
suit a modular world.

David
-----


> -Ulf
>
>
> Am 08.05.2012 09:36, schrieb david.holmes at oracle.com:
>> Changeset: 48513d156965
>> Author: dholmes
>> Date: 2012-05-08 02:59 -0400
>> URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/48513d156965
>>
>> 7103570: AtomicIntegerFieldUpdater does not work when SecurityManager
>> is installed
>> Summary: Perform class.getField inside a doPrivileged block
>> Reviewed-by: chegar, psandoz
>>
>> !
>> src/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java
>>
>> !
>> src/share/classes/java/util/concurrent/atomic/AtomicLongFieldUpdater.java
>> !
>> src/share/classes/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.java
>>
>> + test/java/util/concurrent/atomic/AtomicUpdaters.java
>>
>>



More information about the core-libs-dev mailing list