RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

[Sean Mullan]

On 11/1/18 3:21 PM, dean.long at oracle.com wrote:
> On 11/1/18 9:48 AM, Sean Mullan wrote:
>> On 11/1/18 1:29 AM, dean.long at 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. 

Yes, I was thinking about using a lambda too, but I'm a bit wary of 
using lambdas in the security checking code. Best to keep the methods 
separate 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.

I would just remove it, so we don't forget about it later.

Looks good otherwise.

--Sean

> http://cr.openjdk.java.net/~dlong/8212605/webrev.3.incr/
> http://cr.openjdk.java.net/~dlong/8212605/webrev.3/
> 
> dl
> 
>> --Sean
>