RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged
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:
>>>> 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.
More information about the core-libs-dev