<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body style="background-color: rgb(255, 255, 255); color: rgb(0, 0,
0);" text="#000000" bgcolor="#FFFFFF">
<br>
<div class="moz-cite-prefix">On 10/04/2014 05:15 AM, Alan Bateman
wrote:<br>
</div>
<blockquote cite="mid:542F6641.3090600@oracle.com" type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->On
03/10/2014 08:13, David M. Lloyd wrote:
<br>
<blockquote type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->:
<br>
<br>
Why the problem occurs
<br>
----------------------
<br>
The root of the problem traces back to
SecurityManager.checkMemberAccess(). This method is the one
remaining method in all of SecurityManager which uses the
calling class context (stack) in order to determine the nature
of the access check that is needed.
<br>
<!--[if !IE]></DIV><![endif]--></blockquote>
Are you sure you see this in JDK 8 too? I ask because I remember
David Holmes changed the Atomic*Updater methods to call
getDeclaredField in a privileged block (JDK-7103570). Also there
work in JDK 8 on caller sensitive methods (JEP 176). As part of
this then SM.checkMemberAccess was deprecated and usages in the
JDK dropped (Class.getDeclaredField and the others no longer use
it).
<br>
<br>
-Alan.
<br>
<br>
<!--[if !IE]></DIV><![endif]--></blockquote>
<br>
Hi,<br>
<br>
It seems that construction-time access checks are fine in JDK9 and
8u20. But I found a corner case where invocation-time access check
is overly restrictive. Take for example the following code:<br>
<br>
package test;<br>
<br>
import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;<br>
<br>
public class Test {<br>
<br>
static class A {<br>
protected volatile int x;<br>
}<br>
<br>
static class B extends A {<br>
static void test() {<br>
A a = new A();<br>
B b = new B();<br>
<br>
b.x = 10; // OK<br>
<br>
a.x = 10; // OK<br>
<br>
AtomicIntegerFieldUpdater<A> xUpdater =<br>
AtomicIntegerFieldUpdater.newUpdater(A.class, "x");
// OK<br>
<br>
xUpdater.set(b, 10); // OK<br>
<br>
xUpdater.set(a, 10); // IllegalAccessException:<br>
// Class test.Test$B can not access a protected member
of class<br>
// test.Test$A using an instance of test.Test$A<br>
}<br>
}<br>
<br>
public static void main(String[] args) {<br>
B.test();<br>
}<br>
}<br>
<br>
<br>
Here we see that Java allows accessing protected field A.x from
class B using either instance of A or instance of B. That's because
B is in the same package as A .<br>
<br>
Atomic*FieldUpdater check is therefore overly restrictive. If the
'protected' modifier is removed from A.x field, the test passes.<br>
<br>
So here's a preview of how this could be fixed:<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~plevart/jdk9-dev/AtomicFieldUpdater.AccessChecks/webrev.01/">http://cr.openjdk.java.net/~plevart/jdk9-dev/AtomicFieldUpdater.AccessChecks/webrev.01/</a><br>
<br>
I have just made a fix for AtomicIntegerFieldUpdater. Other
Atomic*FieldUpdaters have similar code. The fix also includes a
simplification of invocation-time access check. In original code,
'cclass' is non-null and equal to 'caller' only in case when the
field is protected and 'caller' class is different from 'tclass',
meaning that fullCheck() is always called in such cases. fullCheck()
in such cases checks that 'obj' is instanceof 'tclass' and 'caller'
at the same time. But in such cases, 'caller' is also a subclass of
'tclass' or else the construction-time checks would fail.<br>
<br>
So I think that we can get away with only one instanceof check. In
patched code the class to check against is equal to 'tclass' if we
are not performing a protected field access check or 'caller' if
protected access is checked. So if we change the meaning of 'cclass'
from:<br>
<br>
this.cclass = (Modifier.isProtected(modifiers)
&&<br>
caller != tclass) ? caller : null;<br>
<br>
to:<br>
<br>
this.cclass = (Modifier.isProtected(modifiers)) ? caller
: tclass;<br>
<br>
we can change the invocation-time check from:<br>
<br>
if (obj == null || obj.getClass() != tclass || cclass !=
null) fullCheck(obj);<br>
<br>
to:<br>
<br>
if (!cclass.isInstance(obj)) failCheck(obj);<br>
<br>
That's the whole check, so fullCheck() becomes failCheck() which
always throws exception (the type of which is determined from the
runtime class of 'obj' instance).<br>
<br>
Now is this check fast enough? It seems so. The Class.isInstance()
is intrinsified. I checked with the following benchmark (the results
on my i7 Linux PC are attached as comments):<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~plevart/jdk9-dev/AtomicFieldUpdater.AccessChecks/AtomicUpdaterBench.java">http://cr.openjdk.java.net/~plevart/jdk9-dev/AtomicFieldUpdater.AccessChecks/AtomicUpdaterBench.java</a><br>
<br>
The 1st and 2nd report should be compared. It seems that get() is
even faster with simplified check while other methods are the same.<br>
<br>
The 3rd report shows a result of experimental
AtomicIntegerFieldUpdater implementation which loads new
VM-anonymous class for each new instance which allows VM compiler to
specialize code for a particular field. Such implementation is
nearly as fast as Java field access. This is just a proof of
concept. A little hack-ish, doesn't include the fix for the overly
restrictive protected access yet, but here it is if anyone is
interested:<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~plevart/jdk9-dev/AtomicFieldUpdater.AccessChecks/AnonClassPerInstance/AtomicIntegerFieldUpdater.java">http://cr.openjdk.java.net/~plevart/jdk9-dev/AtomicFieldUpdater.AccessChecks/AnonClassPerInstance/AtomicIntegerFieldUpdater.java</a><br>
<br>
<br>
Regards, Peter<br>
<br>
</body>
</html>