RFR: 8310874: Runthese30m crashes with klass should be in the placeholders during verification [v3]

[Coleen Phillimore]

On Fri, 18 Aug 2023 13:31:59 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This is a better fix and cleanup to make placeholder use for non-parallel capable class loaders the same as the other sort of class loaders, and fixes potential problems with thread safety in the definer field of the PlaceholderEntry.   It's not easy to backport, which is why I chose the other fix for this bug.
>> Tested with tier1-4 with and without injected LoaderConstrantTable::verify code.
>
> Coleen Phillimore has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - Merge branch 'master' into placeholder-bug
>  - Don't need atomics. thanks for the explanation, David.
>  - Removed comments and unnecessary accessors.
>  - 8310874: Runthese30m crashes with klass should be in the placeholders during verification
>  - 8310874: Runthese30m crashes with klass should be in the placeholders during verification

@iklam pointed out to me privately that SystemDictionary::load_instance_class has the same pattern and the placeholder tokens are not consistent here either.  For LOAD_INSTANCE (ie loading), we only take a placeholder token for the bootstrap loader and for non-parallel capable class loaders, and NOT for parallel capable class loaders.
For some reason, I can't provoke this failure with a LoaderConstraintsTable::verify() but it appears possible.
The verify in LoaderConstraintsTable is trying to make sure there are no InstanceKlass in the table that aren't loaded or in the process of being loaded.  It doesn't do a very good job.  If there were such InstanceKlass though, the memory would be reclaimed and the access to that InstanceKlass (ie, if there were an error) would crash.  So the verification code doesn't really find anything useful the way it's written.  I'm going to go back to my first patch that removes this.

FYI, I like this patch for other reasons but not for fixing this bug.

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

PR Comment: https://git.openjdk.org/jdk/pull/14889#issuecomment-1689841562
PR Comment: https://git.openjdk.org/jdk/pull/14889#issuecomment-1689843711