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

[David Holmes]

On 8/05/2012 8:22 PM, David Holmes wrote:
> <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.

I was forgetting there's another major consideration here too, and that 
is that this code sync's up with Doug Lea's JSR-166 CVS repository. The 
changes I made are independent of the JDK used, but if I refactored 
things into ReflectUtil then the code would only be valid on a JDK with 
that update - which means Doug would need to maintain a different 
version of this fix for older JDKs.

David
-----

> 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
>>>
>>>