8161129 Unsafe::getUnsafe should allow the platform class loader to access it

[Peter Levart]

Just a thought...

Is it already too late (or too early?) to consider for 
jdk.internal.misc.Unsafe to expose a static instead of instance API ?

Having instance API was a clever way to fold access checks to static 
initializer of consumer class, but with Jigsaw, this is not needed any 
more, right?

Regards, Peter

On 07/19/2016 09:58 AM, Paul Sandoz wrote:
>> On 19 Jul 2016, at 04:07, Martin Buchholz <martinrb at google.com> wrote:
>>
>> Recent discussion here:
>>
>> http://markmail.org/search/?q=core-libs-dev%20ConcurrentHashMap%20jdk.internal.misc.unsafe#query:core-libs-dev%20ConcurrentHashMap%20jdk.internal.misc.unsafe+page:1+mid:iviymmlotcuasv6t+state:results
>>
>> There are too many places where reflective code can get its hands on a jdk.internal.misc.Unsafe.  They can even call its toString method, but (perhaps!) not the dangerous methods.
>>
> Right, and even synchronise on it :-) since they are methods on Object. Access control will block reflective invocation on the Unsafe methods for unqualified exports.
>
> It’s Remi’s last point that clinched it for me, i have removed:
>
>    Reflection.registerFieldsToFilter(Unsafe.class, "theInternalUnsafe");
>
> Paul.
>
>
>
>> On Mon, Jul 18, 2016 at 10:29 AM, Remi Forax <forax at univ-mlv.fr> wrote:
>> Hi Paul,
>>
>> ----- Mail original -----
>>> De: "Paul Sandoz" <paul.sandoz at oracle.com>
>>> À: "Core-Libs-Dev" <core-libs-dev at openjdk.java.net>
>>> Envoyé: Lundi 18 Juillet 2016 18:06:44
>>> Objet: 8161129 Unsafe::getUnsafe should allow the platform class loader to    access it
>>>
>>> Hi,
>>>
>>> Please review the patch below.
>>>
>> [...]
>>
>>> I also took the opportunity to hide the “theInternalUnsafe” field in the
>>> sun.misc.Unsafe. That just avoids any futile attempts to obtain the field’s
>>> value after which any reflective access on that value will fail.
>> I see 3 good reasons to not do that,
>> - as you said it is futile to try to get a reference to jdk.internal.misc.Unsafe because you will never be able to call a method on it.
>> - the code you add contains a string that reference a field name which is erro prone when doing a refactoring
>> - jdk.internal.misc.Unsafe is stored in static fields of a lot of exported classes, so why trying to 'protect' only sun.misc.Unsafe.
>>
>>> Paul.
>> Rémi
>>
>>
>>> diff -r 4f5f82c457af src/jdk.unsupported/share/classes/sun/misc/Unsafe.java
>>> --- a/src/jdk.unsupported/share/classes/sun/misc/Unsafe.java  Mon Jul 18
>>> 13:13:52 2016 +0800
>>> +++ b/src/jdk.unsupported/share/classes/sun/misc/Unsafe.java  Mon Jul 18
>>> 17:50:20 2016 +0200
>>> @@ -56,6 +56,7 @@
>>>
>>>       static {
>>>           Reflection.registerMethodsToFilter(Unsafe.class, "getUnsafe");
>>> +        Reflection.registerFieldsToFilter(Unsafe.class,
>>> "theInternalUnsafe");
>>>       }
>>>
>>>       private Unsafe() {}
>>>