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 [email protected]:
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
