RFR (XXS): 7110173: GCNotifier::pushNotification publishes stale data.
john.cuthbertson at oracle.com
Tue Nov 15 09:33:23 PST 2011
Thanks for the review. I did and I did that consciously. As you say -
with the current code the listener would only be notified if the
_recordPostGCUsage field of the TraceMemoryManagerStats object is true
(which it is by default) and none of the TraceMemoryManagerStats
instances created by the collectors change this. But there's nothing to
stop a collector creating a TraceMemoryManagerStats object with
_recordPostGCUsage false and_recordGCEndTime true. In this case wouldn't
we still want to notify the listener? I may be wrong (in which case I'll
add the extra guard) but I believe that recording (some) data and
notification should be two independent operations.
On 11/14/11 22:35, David Holmes wrote:
> Hi John,
> On 15/11/2011 4:46 AM, John Cuthbertson wrote:
>> Can I have a couple of volnteers to review the fix for this CR? The
>> webrev can be found at:
> In the original code the pushNotification is conditional on
> recordPostGCUsage, but you've removed that guard by moving the code.
>> The issue here was that the routine GCNotifier::pushNotification(),
>> which uses GC data held in GCMemoryManager::_last_gc_stat, was being
>> called before the values in GCMemoryManager::_last_gc_stat were being
>> populated for the current GC. As a result the JVM could pass
>> uninitialized or stale data to a listener. The fix is to move the call
>> to GCNotifier::pushNotification() after the code that populates
>> GCMemoryManager::_last_gc_stat. I also modified the GCStatInfo
>> constructor to fully initialize instances of that class.
>> Testing: The supplied test case on Windows, a crafted test case on
>> Solaris, and the nsk GC tests.
More information about the hotspot-gc-dev