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.hol...@oracle.com>; Shafi Ahmad <shafi.s.ah...@oracle.com> Cc: serviceability-dev@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.ah...@oracle.com";<shafi.s.ah...@oracle.com> *Cc:* HYPERLINK "mailto:serviceability-dev@openjdk.java.net"serviceability-dev@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
