RFR: 8217450: Add PackageEntry::locked_lookup_only

[Lois Foltan]

On 1/22/2019 2:35 PM, Claes Redestad wrote:
>
>
> 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?

Hi Claes,
No you're not!  I wrongfully assumed you had not changed the conditional 
of the assert and was only checking if pkg != NULL. Thanks for 
clarifying, I'm ok with the change.
Lois

>
> /Claes