RFR: 8259839 SystemDictionary exports too much implementation [v2]

[Coleen Phillimore]

On Wed, 27 Jan 2021 01:07:50 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> src/hotspot/share/classfile/systemDictionary.cpp line 1748:
>> 
>>> 1746:     k->class_loader_data()->add_to_deallocate_list(k);
>>> 1747:   } else if (HAS_PENDING_EXCEPTION) {
>>> 1748:     assert(defined_k != NULL, "Should not have a klass if there's an exception");
>> 
>> Should the assert be `defined_k == NULL`? I wonder if we have a test case that covers this branch.
>
> Yes, it should be.  That's a cut/paste error on my part.  There is a test for this: runtime/Metaspace/DefineClass.java

Changing it to defined_k == NULL found a bug in a previous refactoring of find_or_define_instance_class.  It should have returned NULL for a pending exception rather than the original k klass.  The old code recreated the exception and had:

  // Can't throw exception while holding lock due to rank ordering
  if (linkage_exception() != NULL) {
    THROW_OOP_(linkage_exception(), NULL); // throws exception and returns
  }

but the new code doesn't clear and recreate the exception.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2247