[10] RFR for JDK-8170299: Debugger does not stop inside the low memory notifications code [internal]

[Shafi Ahmad]

Thank you Mandy and David for review comments.

 

Please find updated webrev: http://cr.openjdk.java.net/~shshahma/8170299/webrev.03/

I have modify the code to use Emitter class rather than Broadcaster.

 

Regards,

Shafi

 

From: mandy chung 
Sent: Tuesday, December 12, 2017 12:06 PM
To: David Holmes <david.holmes at oracle.com>; Shafi Ahmad <shafi.s.ahmad at oracle.com>
Cc: serviceability-dev at openjdk.java.net
Subject: Re: [10] RFR for JDK-8170299: Debugger does not stop inside the low memory notifications code [internal]

 

 

On 12/11/17 9:32 PM, David Holmes wrote:

Hi Shafi, 

I missed the first round of reviews on this so hadn't seen previous discussion. I'm still uneasy about introducing a new thread to the mix here, but at least Mandy's suggestion to modify the *Emitter class rather than change to the *Broadcaster is a lot less disruptive: 

http://mail.openjdk.java.net/pipermail/serviceability-dev/2017-September/021851.html 


Yes this is a better alternative (thanks for bringing this up, David).

Mandy




David 

On 12/12/2017 3:02 PM, Shafi Ahmad wrote: 



Thank you Mandy and David. 

I am sorry, somehow I missed your earlier comment regarding the RFE. 

 >> This is a spec change.  Is this intentional? 

Yes, this change is required as this is referenced inside  MemoryImpl.java. 

I will file a separate RFE for adding hasListeners(). 

I will send the updated webrev after incorporating the review comment. 

Regards, 

Shafi 

*From:*mandy chung 
*Sent:* Tuesday, December 12, 2017 2:35 AM 
*To:* Shafi Ahmad HYPERLINK "mailto:shafi.s.ahmad at oracle.com"<shafi.s.ahmad at oracle.com> 
*Cc:* HYPERLINK "mailto:serviceability-dev at openjdk.java.net"serviceability-dev at openjdk.java.net 
*Subject:* Re: [10] RFR for JDK-8170299: Debugger does not stop inside the low memory notifications code [internal] 

On 12/11/17 2:31 AM, Shafi Ahmad wrote: 

    The method hasListeners() is referenced  inside MemoryImpl.java and 
    defined in  NotificationEmitterSupport. 

    This method is not present in  NotificationBroadcasterSupport so I 
    added it to NotificationBroadcasterSupport. 


This is a spec change.  Is this intentional?  I replied in Sept in reviewing an earlier version [1] that this cannot be backported.  If you intend to make this spec change, it's better to separate this RFE and it requires CSR. 


    Jdk10 bug: https://bugs.openjdk.java.net/browse/JDK-8170299 

    Webrev link: http://cr.openjdk.java.net/~shshahma/8170299/webrev.02/ 
    HYPERLINK "http://cr.openjdk.java.net/%7Eshshahma/8170299/webrev.02/"<http://cr.openjdk.java.net/%7Eshshahma/8170299/webrev.02/> 


MemoryImpl.java 

189                     th.setName("Debugger"); 

this is not a debugger thread.  Maybe rename it to 

"MemoryPool notification thread"? 

Mandy 

[1] http://mail.openjdk.java.net/pipermail/serviceability-dev/2017-September/021836.html

 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20171213/4807609f/attachment.html>