[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    RE: Buggy exception handling in ServerNotifForwarder#removeNotificationListener
From:       Kevin Walls <kevin.walls () oracle ! com>
Date:       2021-08-20 14:47:32
Message-ID: MN2PR10MB3278BBEDD4A5FE3943D23D2F97C19 () MN2PR10MB3278 ! namprd10 ! prod ! outlook ! com
[Download RAW message or body]

I logged this: https://bugs.openjdk.java.net/browse/JDK-8272780


-----Original Message-----
From: Kevin Walls 
Sent: 19 August 2021 10:58
To: Andrey Turbanov <turbanoff@gmail.com>; serviceability-dev@openjdk.java.net
Subject: RE: Buggy exception handling in \
ServerNotifForwarder#removeNotificationListener

Hi -

Yes, looks like we intend to keep the first Exception thrown, and throw that after \
the loop, but not to stop the loop attempting all removeNotificationListener() calls. \
So it would make sense to assign the caught Exception to re, only if re IS null. 

So currently this method never throws an Exception.  Let me know if you've tested the \
change or plan to log a bug.  We should proceed with care in case this change causes \
any surprises, as it  has been like this forever.

Thanks
Kevin


-----Original Message-----
From: serviceability-dev <serviceability-dev-retn@openjdk.java.net> On Behalf Of \
                Andrey Turbanov
Sent: 18 August 2021 12:52
To: serviceability-dev@openjdk.java.net
Subject: Buggy exception handling in ServerNotifForwarder#removeNotificationListener

Hello.

During investigation of IDEA inspections I found suspicious code in a method \
com.sun.jmx.remote.internal.ServerNotifForwarder#removeNotificationListener \
https://github.com/openjdk/jdk/blob/master/src/java.management/share/classes/com/sun/jmx/remote/internal/ServerNotifForwarder.java#L167


Exception re = null;
for (int i = 0 ; i < listenerIDs.length ; i++) {
    try {
        removeNotificationListener(name, listenerIDs[i]);
    } catch (Exception e) {
        // Give back the first exception
        //
        if (re != null) {
            re = e;
        }
    }
}
if (re != null) {
    throw re;
}

Variable 're' set initially to 'null', but then in a catch block it checked to be '!= \
null'. As you can see this condition can never be true. And exceptions from inner \
calls are not propagated as expected. It seems that it should check if (re == null) \
inside the catch block.


Andrey Turbanov


[prev in list] [next in list] [prev in thread] [next in thread]