Check for duplicate class definitions when creating rom class

[dnakamura]

Fixes #15166

Signed-off-by: Devin Nakamura devinn@ca.ibm.com

[dnakamura]

@gacholio can you take a look at this? I'm not sure if putting the check in during rom class creation causes us any issues anywhere else, but it seemed the best place to bail out as early as possible.

[gacholio]

At a glance, I dislike this being in the ROM code since it's calling out to hashClassTableAt. The new code also only comes into play if no name is provided from defineClass - is the case already handled when the name is provided?

[dnakamura]

At a glance, I dislike this being in the ROM code since it's calling out to hashClassTableAt. The new code also only comes into play if no name is provided from defineClass - is the case already handled when the name is provided?

I am also not crazy about putting this in the rom code, however the other option is to do the check after the rom class is created, but that means wasting time creating a rom class which is just going to be thrown away.

In the case where a name is provided the check is performed early in the jcl before attempting to load the class (

openj9/runtime/jcl/common/jcldefine.c

Line 161 in cddb8bf

vmFuncs->setCurrentExceptionNLSWithArgs(currentThread, J9NLS_JCL_DUPLICATE_CLASS_DEFINITION, J9VMCONSTANTPOOL_JAVALANGLINKAGEERROR, utf8Length, utf8Name);

)

[gacholio]

Although I would consider this to be an error situation that really shouldn't come up in production, we've seen some pretty bad java code in the past, so avoiding creating ROM classes that never go away is probably a good idea (ROM classes are never discarded until the loader is unloaded).

[gacholio]

@tajila Are you OK with this (the change is appealingly simple where it is). The NULL check on the javaVM should allow cfdump to continue working.

[gacholio]

While you're here, the formatting here is off:

https://github.com/eclipse-openj9/openj9/blob/master/runtime/jcl/common/jcldefine.c#L136-L137

In the case where a name is not specified, this code appears to call the hash code with NULLs which should crash (nothing to do with your change):

openj9/runtime/jcl/common/jcldefine.c

Line 157 in cddb8bf

if (NULL != vmFuncs->hashClassTableAt(classLoader, utf8Name, utf8Length)) {

[tajila]

I think we need to do something like this to avoid throwing the linkage error after creating the ROM class in cases where no name is passed in. I can't see any alternative that doesn't involve some kind of class file parsing.

[dnakamura]

PR updated

[dnakamura]

fixed

[gacholio]

jenkins test sanity zlinux jdk11

[gacholio]

I think we need to do something like this to avoid throwing the linkage error after creating the ROM class in cases where no name is passed in. I can't see any alternative that doesn't involve some kind of class file parsing.

@tajila Are you still concerned about anything? If not, I'll merge after testing completes.

[tajila]

@tajila Are you still concerned about anything? If not, I'll merge after testing completes.

LGTM