8144979: Context.fromClass should catch exception from Class.getClassLoader call

[Sundararajan Athijegannathan]

Hi,

Inline comments below...


On 12/9/2015 2:08 PM, Attila Szegedi wrote:
> So… I presume the security exception only thrown when a SecurityManager is
> present?

Yes. But note that we run all the tests under a security manager (except 
for tests under "test/script/nosecurity" directory)

> This is actually somewhat weird, especially since VM anonymous
> classes should return their host class' loader as their loader.
> getClassLoader() doc says
>
> If a security manager is present, and the caller's class loader is not null

Well, for unknown reason,  this fails in jake/nashorn - but not on 
jdk9-dev/nashorn. I'm trying to come up with a standalone test to 
reproduce it - but not
successful so far.
>> and the caller's class loader is not the same as or an ancestor of the
>> class loader for the class whose class loader is requested, then this
>> method calls the security manager's checkPermission method with a
>> RuntimePermission("getClassLoader") permission
>
> I'd think that for Context.class.classLoader it is true that it is either
> null or the ancestor of the script's class loader, is that not so?

Nashorn's loader is extension loader. And it is ancestor of script 
loader. because script loader uses thread context loader as parent - and 
thread context loader's parent is extension loader in this case.


> I'm just
> trying to understand *why* can this SecurityException arise at all.

As I said, tests are run under security manager and tests do fail in 
jake/nashorn without this change. As for the root cause, I/we need to 
understand by coming up with an independent test case (yet).
This workaround fix is for two reasons:

1) Will make future jdk9-> jake merge would be clean (for this file/method)
2) When jake merges to jdk9 in future, we don't want failure in this 
part of code to disrupt nashorn test run. Because this is just an 
optimization (to get Context from loader instance). We should be able
to get current Context from thread-local in any case. i.e., should not 
fail/disrupt test run for this reason.

Thanks,
-Sundar

> Attila.
>
>
> On Wed, Dec 9, 2015 at 9:20 AM, Michael Haupt <michael.haupt at oracle.com>
> wrote:
>
>> Hi Sundar,
>>
>> lower-case thumbs up.
>>
>> One remark: I'm a bit concerned about the plethora of files involved with
>> the dynalink samples. For each particular sample, there's a Java file, a
>> sample JS file, and a linker JS file, where the linker compiles the Java
>> file and assembles a JAR. The linkers all look pretty much the same. In my
>> opinion, providing one script that takes care of compilation and JARring
>> and is loaded from all actual samples would keep the samples more lean.
>>
>> Best,
>>
>> Michael
>>
>>> Am 09.12.2015 um 08:56 schrieb Sundararajan Athijegannathan <
>> sundararajan.athijegannathan at oracle.com>:
>>> Please review http://cr.openjdk.java.net/~sundar/8144979/ for
>> https://bugs.openjdk.java.net/browse/JDK-8144979
>>> This fix is already in jake/nashorn. See also:
>> https://bugs.openjdk.java.net/browse/JDK-8144568
>>> In addition, I've moved all dynalink samples to
>> $nashorn/samples/dynalink directory and renamed the README.
>>> Thanks,
>>> -Sundar
>> --
>>
>>   <http://www.oracle.com/>
>> Dr. Michael Haupt | Principal Member of Technical Staff
>> Phone: +49 331 200 7277 | Fax: +49 331 200 7561
>> Oracle Java Platform Group | LangTools Team | Nashorn
>> Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam,
>> Germany
>>   <http://www.oracle.com/commitment>     Oracle is committed to developing
>> practices and products that help protect the environment
>>
>>