RFR: 8061244 Use of stack-less ClassNotFoundException thrown from (URL)ClassLoader.findClass()

[Stanimir Simeonoff]

I like the changes, esp the black listing which is long overdue in JDK
(middleware containers have been doing that for over a decade).

Not sure if there is any specification governing if two consecutive
attempts to load the same class name may respectively fail and succeed - if
the black listing passes, I believe that should go to classloading spec.

Adding the initiating classloader in the Thread doesn't pose security risks
the way it's implemented, so it's fine imo.

Stanimir

On Tue, Oct 21, 2014 at 1:37 PM, Peter Levart <peter.levart at gmail.com>
wrote:

> Hi,
>
> I guess we are not happy with stack-traces that only reveal call stack up
> to the public ClassLoader.loadClass() method and hide everything happening
> after that even if paired with a more descriptive message. So I improved
> the strategy of throwing stack-less exception. New strategy throws
> full-stack exception from findClass() if findClass() is invoked on the
> "initiating" class loader and stack-less exception otherwise. How can we
> know if a particular ClassLoader instance is the initiating class loader
> for a particular class loading request? VM knows that when invoking
> Class.forName() or when VM triggers class loading, because such calls are
> wired through VM or initiate from VM. On the Java side, we can establish a
> similar thread-local context at the entry point of Class.loadClass() public
> method.
>
> This strategy gives normal stack traces for exceptions that escape the
> ClassLoader.loadClass() public method and avoids creating stack-full
> exceptions that are later swallowed.
>
> That's not all. I have another optimization at hand. Parallel capable
> class loaders maintain a ConcurrentHashMap of Objects used as locks to
> synchronize on when executing class loading request for a particular class
> name. These seem like a waste of resource and can be used for more than
> just locks. My 1st attempt at (ab)using these objects is the following:
>
> http://cr.openjdk.java.net/~plevart/jdk9-dev/ClassLoader.CNFE/webrev.03/
>
> Here a boolean flag is added to such Lock object that indicates whether a
> class loading attempt has already been performed for a particular class
> name. The 1st attempt at loading a particular class can therefore skip
> calling findLoadedClass() native method. This has a measurable performance
> impact as indicated in the following benchmark:
>
> http://cr.openjdk.java.net/~plevart/jdk9-dev/ClassLoader.
> CNFE/CLBench2.java
>
> This is a variant of previous benchmark - only the part that measures
> successful attempts at loading a class with a child of extension class
> loader, but does that at different call-stack depths (10, 20, 40, 80, 160
> frames). Original JDK 9 code gives these results:
>
>     Benchmark                                Mode  Samples  Score   Error
> Units
>     j.t.CLBench2.loadNewClassSuccessSS010      ss        5  3.947 ±
> 0.080      s
>     j.t.CLBench2.loadNewClassSuccessSS020      ss        5  4.001 ±
> 0.119      s
>     j.t.CLBench2.loadNewClassSuccessSS040      ss        5  4.068 ±
> 0.115      s
>     j.t.CLBench2.loadNewClassSuccessSS080      ss        5  4.323 ±
> 0.061      s
>     j.t.CLBench2.loadNewClassSuccessSS160      ss        5  4.633 ±
> 0.118      s
>
>
> Doing stack-less CNF exceptions shows that call-stack depth does not have
> an impact any more:
>
>     Benchmark                                Mode  Samples  Score   Error
> Units
>     j.t.CLBench2.loadNewClassSuccessSS010      ss        5  3.753 ±
> 0.155      s
>     j.t.CLBench2.loadNewClassSuccessSS020      ss        5  3.755 ±
> 0.160      s
>     j.t.CLBench2.loadNewClassSuccessSS040      ss        5  3.767 ±
> 0.124      s
>     j.t.CLBench2.loadNewClassSuccessSS080      ss        5  3.762 ±
> 0.148      s
>     j.t.CLBench2.loadNewClassSuccessSS160      ss        5  3.781 ±
> 0.146      s
>
>
> Adding findLoadedClass() avoidance on top of that gives even better
> results:
>
>     Benchmark                                Mode  Samples  Score   Error
> Units
>     j.t.CLBench2.loadNewClassSuccessSS010      ss        5  3.539 ±
> 0.185      s
>     j.t.CLBench2.loadNewClassSuccessSS020      ss        5  3.548 ±
> 0.165      s
>     j.t.CLBench2.loadNewClassSuccessSS040      ss        5  3.586 ±
> 0.226      s
>     j.t.CLBench2.loadNewClassSuccessSS080      ss        5  3.541 ±
> 0.053      s
>     j.t.CLBench2.loadNewClassSuccessSS160      ss        5  3.570 ±
> 0.113      s
>
>
>
>
> Regards, Peter
>
>
> On 10/16/2014 04:05 PM, Peter Levart wrote:
>
>> I created an issue in Jira to track this investigation:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8061244
>>
>> The latest webrev of proposed patch is still:
>>
>> http://cr.openjdk.java.net/~plevart/jdk9-dev/ClassLoader.CNFE/webrev.02/
>>
>> I invite the public to review it.
>>
>> Thanks.
>>
>> Peter
>>
>> On 10/16/2014 10:44 AM, Peter Levart wrote:
>>
>>> Hi,
>>>
>>> As for usage of SharedSecrets, they are not needed to access CNFE
>>> package-private constructor from java.lang.ClassLoader, since they are in
>>> the same package, and from java.net.URLClassLoader, the call to
>>> super.findClass(name) does the job. So here's an updated webrev that also
>>> includes more descriptive message:
>>>
>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/ClassLoader.CNFE/webrev.02/
>>>
>>>
>>> Regards, Peter
>>>
>>> On 10/16/2014 09:35 AM, Peter Levart wrote:
>>>
>>>> On 10/16/2014 01:49 AM, Stanimir Simeonoff wrote:
>>>>
>>>>> First, really nice tests.
>>>>>
>>>>> As for hiding: missing the real classloader impl. might be quite a
>>>>> bumper
>>>>> for some middleware implementations. That would make pretty hard to
>>>>> trace
>>>>> dependency issues without explicit logging, the latter is usually
>>>>> available
>>>>> but still. Also it'd depend if the classloaders actually use
>>>>> super.findClass() or not.
>>>>> IMO, the option should be switchable via some system property.
>>>>>
>>>>
>>>> With a little tweak, the message of the stack-less exception thrown
>>>> from findClass() methods can be extended to include the classloader's
>>>> runtime class name and this message can be inherited by a replacement
>>>> stack-full exception. So the stack-trace would look like:
>>>>
>>>> Exception in thread "main" java.lang.ClassNotFoundException: XXX
>>>> (thrown by: sun.misc.Launcher$AppClassLoader)
>>>>     at java.lang.ClassLoader.loadClass(ClassLoader.java:366)
>>>>     at java.lang.Class.forName0(Native Method)
>>>>     at java.lang.Class.forName(Class.java:265)
>>>>     at Test.doIt(Test.java:7)
>>>>     at Test.main(Test.java:11)
>>>>
>>>> Instead of what we have now:
>>>>
>>>> Exception in thread "main" java.lang.ClassNotFoundException: XXX
>>>>     at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
>>>>     at java.lang.ClassLoader.loadClass(ClassLoader.java:426)
>>>>     at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:317)
>>>>     at java.lang.ClassLoader.loadClass(ClassLoader.java:359)
>>>>     at java.lang.Class.forName0(Native Method)
>>>>     at java.lang.Class.forName(Class.java:265)
>>>>     at Test.doIt(Test.java:7)
>>>>     at Test.main(Test.java:11)
>>>>
>>>>
>>>> Would this be enough to cover your concern?
>>>>
>>>> Regards, Peter
>>>>
>>>>
>>>>> I am not sure about the need to hide the stackless c-tor as the effect
>>>>> can
>>>>> be achieved by overriding fillInStackTrace(); w/o the extra baggage of
>>>>> JavaLangAccess.
>>>>>
>>>>> Overall very decent improvement.
>>>>>
>>>>> Cheers
>>>>> Stanimir
>>>>>
>>>>>
>>>>>
>>>
>>
>