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 <[email protected]> On Behalf Of Andrey Turbanov Sent: 18 August 2021 12:52 To: [email protected] 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
