JDK 8 RFR 7179567: JCK8 tests: api/java_net/URLClassLoader/index.html#Ctor3 failed with NPE

[David Holmes]

Hi Brian,

Aside: I'm confused about the relationship of this bug and JDK-6445180 - 
are they not duplicates? Seems to me this one should have been closed as 
a dup when it was reported.

On 5/10/2013 6:58 AM, Brian Burkhalter wrote:
> On Oct 3, 2013, at 5:38 PM, Brian Burkhalter wrote:
>
>> On Oct 3, 2013, at 5:35 PM, Alan Bateman wrote:
>>
>>> On 03/10/2013 16:10, Brian Burkhalter wrote:
>>>> Please review and comment at your convenience.
>>>>
>>>> Issue:	https://bugs.openjdk.java.net/browse/JDK-7179567
>>>> Webrev:	http://cr.openjdk.java.net/~bpb/7179567/
>
> An updated webrev which I hope adequately addresses the expressed concerns may be found at:
>
> http://cr.openjdk.java.net/~bpb/7179567.2/

URLClassLoader.java: please delete the commented out line:

+         //Objects.requireNonNull(urls);

SecureClassLoader.java:

186      * @throws SecurityException if the {@code ClassLoader} instance 
is not
  187      * initialized.

should read "this classloader instance" as it refers to the current 
instance. Also this may be bringing the spec into line with the 
implementation but "initialization" here is purely an implementation 
detail not part of the specification for this class so it should not be 
mentioned explicitly - and this change still needs a CCC (which might 
help determine exactly what should be said here - I'm unclear how you 
can try to call getPermissions if this is uninitialized as it would need 
'this' to escape from the constructor - which perhaps it can do via a 
third-party security manager?).

NoCallStackClassLoader.java: the comment is far too verbose, we don't 
write such explanatory kinds of comments for this kind of thing (else 
the code would be overrun with commentary!).

Aside: if you are a JDK Author you don't need a Contributed-by line in 
the commit comments.

David
-----

>>
>>> Will you be adding tests for these cases to the webrev?
>>
>> As needed once the concept in general is accepted.
>
> The foregoing webrev includes a test of the affected public methods.
>
> Thanks,
>
> Brian
>