On 11/1/18 9:48 AM, Sean Mullan wrote:
On 11/1/18 1:29 AM, dean.l...@oracle.com wrote:
On 10/31/18 9:39 PM, Bernd Eckenfels wrote:
http://cr.openjdk.java.net/~dlong/8212605/webrev.1/src/java.base/share/classes/java/security/AccessController.java.udiff.html

In checkContext should the security manager be null checked first instead of last to optimize for the typical case? (If the side effects in that expression are desired it should be documented)

I was following the example of createWrapper.  The side-effects of getInnocuousAcc() will only happen once, so the order shouldn't matter here, except for performance reasons.  I don't have a strong opinion about the order, but it looks like the typical case for createWrapper would also be the typical case for checkContext, so maybe they both should be changed, if indeed a null security manager is the more typical case.

A null SM should be the more common case. I am ok with changing the order so the SM is checked first, but it should be done in both the createWrapper and checkContext methods. Alternatively, we could see if they could be combined to eliminate the duplicate code but that might not be practical from looking at them.


I don't see a way to do it without using a lambda or doing extra work in one version, so I just changed the order for now.  I also replaced getCallerPD with the faster getProtectionDomain and removed a stale comment about impliesCreateAccessControlContext being called by the VM.  It should be safe to remove now, but I left it in to minimize changes.

http://cr.openjdk.java.net/~dlong/8212605/webrev.3.incr/
http://cr.openjdk.java.net/~dlong/8212605/webrev.3/

dl

--Sean

Reply via email to