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

[Zhengyu Gu]

On 06/28/2016 01:52 PM, Lois Foltan wrote:
>
> On 6/28/2016 11:07 AM, Zhengyu Gu wrote:
>> Hi Lois,
>>
>> The comments in PackageEntry::delete_qualified_exports() suggests 
>> that it is related to a transition from qualified exports to 
>> unqualified.
>
> That is correct.  A package can transition from being qualifiedly 
> exported, to unqualifiedly exported. Its exported scope is widened. A 
> package can not transition from being unqualifiedly exported to all 
> modules to being specifically qualified to one or more modules.
>
>>
>> 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.
>
> We chose not to delete the qualified exports at that point but to 
> delay that until the next GC safepoint.  When a package does 
> transition from being qualifiedly exported to unqualifiedly exported, 
> its qualified exports list is moved to the _exported_pending_delete 
> field which is then deleted later, via a call to 
> PackageEntry::delete_qualified_exports().  That deletion of the 
> _exported_pending_delete list does occur either during the purge walk 
> or if the class loader that the package is defined to dies.
I see. Thanks for the explanation.

-Zhengyu



>
> Thanks,
> Lois
>
>>
>> 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
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>