RFR (M): JDK-8159262: Walking PackageEntry Export and ModuleEntry Reads Must Occur Only When Neccessary And Wait Until ClassLoader's Aliveness Determined

[Zhengyu Gu]

Hi Lois,

The comments in PackageEntry::delete_qualified_exports() suggests that 
it is related to a transition from qualified exports to unqualified.

It seems to me that, when overriding package's exports from qualified to 
unqualified, ex. via JVM_AddModuleExportsToAll(), delete_qualified_exports()
should be called. I don't see it has anything to do with class unloading.

Thanks,

-Zhengyu



On 06/28/2016 10:11 AM, Lois Foltan wrote:
>
> On 6/28/2016 9:28 AM, Zhengyu Gu wrote:
>> Thanks for the changes.
>>
>> PackageEntry::delete_qualified_exports() has nothing to do with class 
>> unloading, should it be handled in earlier loop? or it may get lost.
>
> Hi Zhengyu,
>
> PackageEntry::delete_qualified_exports() have everything to do with 
> class unloading and is a direct consequence of a loader dying.  When a 
> class loader dies, its ModuleEntryTable and PackageEntryTable are 
> deleted, see ClassLoaderData's destructor. In particular when a 
> PackageEntryTable is deleted, each destructor is called for every 
> package entry within that table which in return involves calling 
> PackageEntry::delete_qualified_exports() if necessary.   That section 
> of code has no bearing on this webrev and the changes in 
> ClassLoaderDataGraph::do_unloading.
>
> Thanks,
> Lois
>
>>
>> -Zhengyu
>>
>> On 06/27/2016 04:50 PM, Lois Foltan wrote:
>>>
>>> On 6/27/2016 12:03 PM, Zhengyu Gu wrote:
>>>> Hi Lois,
>>>>
>>>> I have a question:
>>>>
>>>> classLoaderData.cpp # 1004 - 1013
>>>>
>>>> If there is no class loader unloaded, (for example, _unloading == 
>>>> NULL), do you still need to execute this purge loop?
>>> Hi Zhengyu,
>>>
>>> Thank you for the review.  I have addressed your concern by moving 
>>> the purge loop into the following conditional that only executes "if 
>>> (seen_dead_loader)", nice catch.  This updated webrev also contains 
>>> an improvement to the test ModuleStress.java to include a module 
>>> defined to a custom system class loader, specified via 
>>> -Djava.system.class.loader.
>>>
>>> http://cr.openjdk.java.net/~lfoltan/bug_jdk8159262.2/webrev/
>>>
>>> Thanks,
>>> Lois
>>>
>>>>
>>>> Thanks,
>>>>
>>>> -Zhengyu
>>>>
>>>> On 06/22/2016 10:51 AM, Lois Foltan wrote:
>>>>>
>>>>> On 6/22/2016 8:56 AM, Alan Bateman wrote:
>>>>>>
>>>>>>
>>>>>> On 22/06/2016 13:51, David Holmes wrote:
>>>>>>>
>>>>>>> How do you envisage any system classloader "dying" ??
>>>>>> The system class loader (be it the built-in application class 
>>>>>> loader or a custom system class loader configured via 
>>>>>> -Djava.system.class.loader) will/can never be GC'ed. 
>>>>>> ClassLoader.getSystemClassLoader() always returns a reference to 
>>>>>> it (for example).
>>>>> Ahh, ok, I was being overly cautious, so no opportunity to reset 
>>>>> the system class loader dynamically at all? 
>>>>> SystemDictionary::_java_class_loader is not initialized until 
>>>>> after the module system initialization is complete. Due to this I 
>>>>> have updated my webrev to check for either the built-in 
>>>>> application class loader or a custom system class loader and have 
>>>>> renamed the method to is_system_class_loader().
>>>>>
>>>>> http://cr.openjdk.java.net/~lfoltan/bug_jdk8159262.1/webrev/
>>>>>
>>>>> Thanks,
>>>>> Lois
>>>>>
>>>>
>>>
>>
>