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

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Shafi Ahmad

Hi,

 

Could you please review the fix for JDK-8170299: Debugger does not stop inside the low memory notifications code.

 

With the current implementation ‘debugger does not stop inside the low memory *notifications* code’

This is expected as the notification code is getting executed with the service thread, which is a hidden thread.

 

shafi@shafi-ahmad:~/Java/jdk8/jdk8u-dev$ jdb -Xmx32m MemoryWarningSystem

Initializing jdb ...

> stop at MemoryWarningSystem$1:25

Deferring breakpoint MemoryWarningSystem$1:25.

It will be set after the class is loaded.

> run

run MemoryWarningSystem

Set uncaught java.lang.Throwable

Set deferred uncaught java.lang.Throwable

>

. . .

> quit

 

Breakpoint not hit.

 

With the attached webrev, jdb stop  at breakpoint inside notification code.

 

shshahma@slc12kkg:/scratch/shshahma/Java/jdk-dev$ jdb -Xmx128m MemoryWarningSystem

Initializing jdb ...

> stop at MemoryWarningSystem$1:25

Deferring breakpoint MemoryWarningSystem$1:25.

It will be set after the class is loaded.

> run

run MemoryWarningSystem

Set uncaught java.lang.Throwable

Set deferred uncaught java.lang.Throwable

>

VM Started: Set deferred breakpoint MemoryWarningSystem$1:25

 

Breakpoint hit: "thread=Debugger", MemoryWarningSystem$1.memoryUsageLow(), line=25 bci=0

25            System.out.println("Memory usage low!!!");

 

Debugger[1] where

  [1] MemoryWarningSystem$1.memoryUsageLow (MemoryWarningSystem.java:25)

  [2] MemoryWarningSystem$2.handleNotification (MemoryWarningSystem.java:48)

  [3] javax.management.NotificationBroadcasterSupport.handleNotification (NotificationBroadcasterSupport.java:284)

  [4] javax.management.NotificationBroadcasterSupport$SendNotifJob.run (NotificationBroadcasterSupport.java:361)

  [5] java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1,135)

  [6] java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:635)

  [7] java.lang.Thread.run (Thread.java:844)

  [8] jdk.internal.misc.InnocuousThread.run (InnocuousThread.java:134)

Debugger[1] list

21   

22        MemoryWarningSystem mws = new MemoryWarningSystem();

23        mws.addListener(new MemoryWarningSystem.Listener() {

24          public void memoryUsageLow(long usedMemory, long maxMemory) {

25 =>         System.out.println("Memory usage low!!!");

26            double percentageUsed = ((double) usedMemory) / maxMemory;

27            System.out.println("percentageUsed = " + percentageUsed);

28            MemoryWarningSystem.setPercentageUsageThreshold(0.8);

29          }

30        });

Debugger[1] threads

Group system:

  (java.lang.ref.Reference$ReferenceHandler)0x362 Reference Handler running

  (java.lang.ref.Finalizer$FinalizerThread)0x361  Finalizer         cond. waiting

  (java.lang.Thread)0x360                         Signal Dispatcher running

Group main:

  (java.lang.Thread)0x1                           main              running

Group InnocuousThreadGroup:

  (jdk.internal.misc.InnocuousThread)0x35f        Common-Cleaner    cond. waiting

  (jdk.internal.misc.InnocuousThread)0x4f1        Debugger          running (at breakpoint)

Debugger[1] step

> Memory usage low!!!

 

Step completed: "thread=Debugger", MemoryWarningSystem$1.memoryUsageLow(), line=26 bci=8

26            double percentageUsed = ((double) usedMemory) / maxMemory;

 

Please see the newly created thread “  (jdk.internal.misc.InnocuousThread)0x4f1        Debugger          running (at breakpoint)”.

 

In the change, the class ‘MemoryImpl’ is derived from class ‘NotificationBroadcasterSupport’ instead of class ‘NotificationEmitterSupport’

NotificationBroadcastSupport takes an Executor, in the constructor of MemoryImpl we are calling super() with an executor, which is a visible thread.

 

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.

 

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

Webrev link: http://cr.openjdk.java.net/~shshahma/8170299/webrev.02/

 

Testing: jprt -testset core, rbt --test nsk.jvmti.testlist,nsk.jdi.testlist.

 

Regards,

Shafi

Reply | Threaded
Open this post in threaded view
|

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

David Holmes
Hi Shafi,

I'm not at all sure about the approach you've taken with this ...

On 11/12/2017 8:31 PM, Shafi Ahmad wrote:

> Hi,
>
> Could you please review the fix for JDK-8170299: Debugger does not stop
> inside the low memory notifications code.
>
> With the current implementation ‘debugger does not stop inside the low
> memory *notifications* code’
>
> This is expected as the notification code is getting executed with the
> service thread, which is a hidden thread.

Okay. I recall some discussion on this problem - the main question being
"why is the service thread executing user-defined Java code?". I'm
assuming now this is an implementation artifact, not part of the main
design of the management framework? But it is unclear. Attempting to
introduce a new thread to execute the code may be reasonable, but raises
  whole range of questions about concurrency, synchronization and
possibly deadlock with this code!

> shafi@shafi-ahmad:~/Java/jdk8/jdk8u-dev$ jdb -Xmx32m MemoryWarningSystem
>
> Initializing jdb ...
>
>  > stop at MemoryWarningSystem$1:25
>
> Deferring breakpoint MemoryWarningSystem$1:25.
>
> It will be set after the class is loaded.
>
>  > run
>
> run MemoryWarningSystem
>
> Set uncaught java.lang.Throwable
>
> Set deferred uncaught java.lang.Throwable
>
>  >
>
> . . .
>
>  > quit
>
> Breakpoint not hit.
>
> With the attached webrev, jdb stop  at breakpoint inside notification code.
>
> shshahma@slc12kkg:/scratch/shshahma/Java/jdk-dev$ jdb -Xmx128m
> MemoryWarningSystem
>
> Initializing jdb ...
>
>  > stop at MemoryWarningSystem$1:25
>
> Deferring breakpoint MemoryWarningSystem$1:25.
>
> It will be set after the class is loaded.
>
>  > run
>
> run MemoryWarningSystem
>
> Set uncaught java.lang.Throwable
>
> Set deferred uncaught java.lang.Throwable
>
>  >
>
> VM Started: Set deferred breakpoint MemoryWarningSystem$1:25
>
> Breakpoint hit: "thread=Debugger",
> MemoryWarningSystem$1.memoryUsageLow(), line=25 bci=0
>
> 25            System.out.println("Memory usage low!!!");
>
> Debugger[1] where
>
>    [1] MemoryWarningSystem$1.memoryUsageLow (MemoryWarningSystem.java:25)
>
>    [2] MemoryWarningSystem$2.handleNotification
> (MemoryWarningSystem.java:48)
>
>    [3]
> javax.management.NotificationBroadcasterSupport.handleNotification
> (NotificationBroadcasterSupport.java:284)
>
>    [4] javax.management.NotificationBroadcasterSupport$SendNotifJob.run
> (NotificationBroadcasterSupport.java:361)
>
>    [5] java.util.concurrent.ThreadPoolExecutor.runWorker
> (ThreadPoolExecutor.java:1,135)
>
>    [6] java.util.concurrent.ThreadPoolExecutor$Worker.run
> (ThreadPoolExecutor.java:635)
>
>    [7] java.lang.Thread.run (Thread.java:844)
>
>    [8] jdk.internal.misc.InnocuousThread.run (InnocuousThread.java:134)
>
> Debugger[1] list
>
> 21
>
> 22        MemoryWarningSystem mws = new MemoryWarningSystem();
>
> 23        mws.addListener(new MemoryWarningSystem.Listener() {
>
> 24          public void memoryUsageLow(long usedMemory, long maxMemory) {
>
> 25 =>         System.out.println("Memory usage low!!!");
>
> 26            double percentageUsed = ((double) usedMemory) / maxMemory;
>
> 27            System.out.println("percentageUsed = " + percentageUsed);
>
> 28            MemoryWarningSystem.setPercentageUsageThreshold(0.8);
>
> 29          }
>
> 30        });
>
> Debugger[1] threads
>
> Group system:
>
>    (java.lang.ref.Reference$ReferenceHandler)0x362 Reference Handler running
>
>    (java.lang.ref.Finalizer$FinalizerThread)0x361  Finalizer        
> cond. waiting
>
>    (java.lang.Thread)0x360                         Signal Dispatcher running
>
> Group main:
>
>    (java.lang.Thread)0x1                           main              running
>
> Group InnocuousThreadGroup:
>
>    (jdk.internal.misc.InnocuousThread)0x35f        Common-Cleaner    
> cond. waiting
>
>    (jdk.internal.misc.InnocuousThread)0x4f1        Debugger  
>         running (at breakpoint)
>
> Debugger[1] step
>
>  > Memory usage low!!!
>
> Step completed: "thread=Debugger",
> MemoryWarningSystem$1.memoryUsageLow(), line=26 bci=8
>
> 26            double percentageUsed = ((double) usedMemory) / maxMemory;
>
> Please see the newly created thread “  
> (jdk.internal.misc.InnocuousThread)0x4f1        Debugger  
>         running (at breakpoint)”.
>
> In the change, the class ‘MemoryImpl’ is derived from class
> ‘NotificationBroadcasterSupport’ instead of class
> ‘NotificationEmitterSupport’

That's quite a significant change and the implications of it are not at
all obvious. The sun.management.NotificationEmitterSupport class
utilises synchronization for thread-safety. The
NotificationBroadcasterSupport class utilises a CopyOnWriteArraylist and
will have quite different concurrency properties. Throw into the mix the
fact you've now added a new thread and we really have no clear idea how
this will all behave.

> NotificationBroadcastSupport takes an Executor, in the constructor of
> MemoryImpl we are calling super() with an executor, which is a visible
> thread.
>
> 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.

You will need to file a CSR request first to add a public method to a
public class. But it's not all clear to me this is a method that should
be added to this API.

I see what you are trying to do, but the impact of these changes seem
quite far reaching and hard to determine.

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

You should note that the bug is not publicly visible.

Thanks,
David

> Webrev link: http://cr.openjdk.java.net/~shshahma/8170299/webrev.02/
>
> Testing: jprt -testset core, rbt --test nsk.jvmti.testlist,nsk.jdi.testlist.
>
> Regards,
>
> Shafi
>
Reply | Threaded
Open this post in threaded view
|

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

Mandy Chung
In reply to this post by Shafi Ahmad


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.


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
Reply | Threaded
Open this post in threaded view
|

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

Shafi Ahmad

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 <[hidden email]>
Cc: [hidden email]
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/

 


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

Reply | Threaded
Open this post in threaded view
|

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

David Holmes
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 <[hidden email]>
> *Cc:* [hidden email]
> *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
>
Reply | Threaded
Open this post in threaded view
|

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

Mandy Chung


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 [hidden email]
*Cc:* [hidden email]
*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


Reply | Threaded
Open this post in threaded view
|

RE: [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 <[hidden email]>; Shafi Ahmad <[hidden email]>
Cc: [hidden email]
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 [hidden email]
*Cc:* [hidden email]
*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

 

Reply | Threaded
Open this post in threaded view
|

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

Mandy Chung


On 12/13/17 2:23 AM, Shafi Ahmad wrote:

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.


It's good that this version does not need the new public API.

The thread factory can be lazily created in the first time to handle a notification.  I think this fix can be made simpler for example, you can add a ListenerInfo::handle method to replace SendNotifJob and the new protected handleNotification method. 

            if (li.filter == null || li.filter.isNotificationEnabled(notification)) {
                NotificationThread.execute(() -> li.handle(notification));
            }

The static Executor field can be moved to NotificationThread (was NotificationThreadFactory) class so that it will be lazily initialized only when there is a notification.

Mandy
Reply | Threaded
Open this post in threaded view
|

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

David Holmes
In reply to this post by Shafi Ahmad
On 13/12/2017 8:23 PM, Shafi Ahmad wrote:
> 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.

Okay. This at least seems less intrusive/disruptive.

Not sure why you added this:

  180     protected void handleNotification(NotificationListener listener,
  181                                       Notification notif, Object
handback) {
  182         listener.handleNotification(notif, handback);
  183     }

instead of executing line 182 directly at 228?

  226         public void run() {
  227             try {
  228                 handleNotification(listenerInfo.listener,
  229                                    notif, listenerInfo.handback);
  230             } catch (Exception e) {

I'm still concerned that the introduction of a new thread to actually
execute the notification will causes more problems than it solves:
- the notifications are no longer synchronous wrt. sendNotification
- the execution context (security credentials etc) may be different in
the InnocuousThread compared to the service thread.
- there may be synchronization/deadlock issues

Despite the spec for NotificationEmitter stating:

"Implementations of this interface can differ regarding the thread in
which the methods of filters and listeners are called."

this may be a significant behavioural change - just to fix a debugger issue.

Thanks,
David

> Regards,
>
> Shafi
>
> *From:*mandy chung
> *Sent:* Tuesday, December 12, 2017 12:06 PM
> *To:* David Holmes <[hidden email]>; Shafi Ahmad
> <[hidden email]>
> *Cc:* [hidden email]
> *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 <[hidden email]>
>         <mailto:[hidden email]>
>         *Cc:* [hidden email]
>         <mailto:[hidden email]>
>         *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/>
>         <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
>