RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged
Sean Mullan
sean.mullan at oracle.com
Thu Nov 1 19:39:41 UTC 2018
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
>
More information about the security-dev
mailing list