RFR: 8217450: Add PackageEntry::locked_lookup_only

[Claes Redestad]

On 2019-01-22 18:10, Lois Foltan wrote:
>>>
>>> Ok - updated http://cr.openjdk.java.net/~redestad/8217450/open.01/ in
>>> place and running some sanity tests before push.
>>>
>>> Thanks!
>>>
>>> /Claes
>>
>> Hi Claes,
>>
>> The code change in modules.cpp lines #211-214 does not look correct. 
>> PackageEntry::locked_create_entry_or_null() will return NULL if the 
>> package exists and has already been created.  The code at lines 
>> #211-214 need a PackageEntry to be successfully returned for java.base 
>> packages created during bootstrapping so we can decrement the refcount 
>> for it appropriately.
> 
> Just to clarify further.  Obtaining a PackageEntry itself is not a 
> requirement for decrementing the refcount of the Symbol* within the 
> table.  However, the assert at line #213 will trigger if 
> PackageEntry::locked_create_entry_or_null() returns NULL for an existing 
> package.  A PackageEntry can be created for any class loaded, such as 
> java/lang/Class and java/lang/Object for example, during bootstrapping 
> before java.base is defined to the JVM.  These classes are put on a 
> fixup_module_list so that once the java.base module is defined the JVM 
> can then fixup their java.lang.Module field to be correctly set to 
> java.base.

Hi Lois,

thanks for looking at this!

As you say: when locked_create_entry_or_null() returns NULL this
*should* be because the package already exists.  The assert is thus not
asserting if pkg == NULL alone, locked_lookup_only() must also return
NULL for the assert to trigger: i.e., I'm checking that a NULL really
meant that the package already existed, and not something else, such as
a failure to allocate the PackageEntry.

I thought this was the purpose of the original assert: getting a NULL
back from locked_create_entry_or_null() when we *know* there was no pre-
existing package should be impossible (we're holding the lock, so that a
NULL must be a failure to allocate or worse).

Am I missing something here?

/Claes