Request for Review (XS): 7143760 Memory leak in GarbageCollectionNotifications

[Frederic Parain]

Here's a new webrev addressing the following issues:
   - the missing HandleMark
   - the clean up of the GCNotificationRequest instance
   - removal of the pending exception testing, now
     exception will be propagated as soon as a method
     returns with a pending exception

http://cr.openjdk.java.net/~fparain/7143760/webrev.01/

Thanks,

Fred

On 2/10/12 11:27 AM, David Holmes wrote:
> On 10/02/2012 7:59 PM, Dmitry Samersoff wrote:
>> Frederic,
>>
>> GCNotificationRequest *request = getRequest();
>>
>> request variable also leaks memory because it will never be deleted on
>> CHECK return path. Could you fix it too?
>
> Further:
>
> 211 JavaCalls::call_virtual(&result,
> 212 gc_mbean_klass,
> 213 vmSymbols::createGCNotification_name(),
> 214 vmSymbols::createGCNotification_signature(),
> 215 &args,
> 216 CHECK);
> 217 if (HAS_PENDING_EXCEPTION) {
> 218 CLEAR_PENDING_EXCEPTION;
> 219 }
> 220
> 221 delete request;
>
> The CHECK at @216 will cause a return if there is an exception pending
> so 217-219 is dead code. This also indicates some confusion about what
> exceptions this method can leave pending. Or it may be that the CHECK at
> #216 was meant to be just THREAD. ??
>
> (Strange this is the second example I've seen of this today!)
>
> David
>
>>
>> -Dmitry
>>
>>
>> On 2012-02-10 13:27, Frederic Parain wrote:
>>> Here's a small fix (one line) for CR 7143760 Memory leak in
>>> GarbageCollectionNotifications
>>>
>>> There's a missing HandleMark at the beginning of the
>>> GCNotifier::sendNotificatin() method. Without this HandleMark, all
>>> handles used when creating GC notifications are kept alive causing a
>>> double leak: in the Java heap and in the thread local handle area of the
>>> service thread.
>>>
>>> Here's the CR:
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7143760
>>> (Warning, the changeset referenced in the CR is not the
>>> one containing the original bug).
>>>
>>> Here's the webrev:
>>> http://cr.openjdk.java.net/~fparain/7143760/webrev.00/
>>>
>>> Thanks,
>>>
>>> Fred
>>>
>>
>>

-- 
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: Frederic.Parain at oracle.com