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
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 <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 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/
<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
