On Fri, 6 Jan 2023 13:11:41 GMT, Kevin Walls <kev...@openjdk.org> wrote:
> Exceptions during setup of these tests could leave e.g. JMXConnector cs as > null. > But, we call cs.close() during a finally block and fail with an NPE, > obscuring the real failure. > We must only call close if this is not null. Thanks Chris - Actually yes the ExceptionTest failure is a little odd... HashedPasswordFileTest more clearly doesn't show an Exception on the way to its finally clause that hits the NPE. The CI failures where there's an NPE at ExceptionTest.java:126, do not include an earlier exception. I see it would/should have been shown by the printThrowable at line 121. Even looking at the full log, there is no earlier exception, so that is odd. If cs is null, I don't see how there isn't an Exception at that point. The newConnectorServer and JMXConnectorFactory.connect methods might give an IOException... but that should have been logged, so it's not clear how we get to that state. (There are ExceptionTest failures in jdk11 and jdk19/panama, but they are the same test.) There are other things that ExceptionTest should have printed if it had got a connection and made progress through the test, and we don't see them, so seems that we failed early on, probably at JMXConnectorFactory.connect(addr); I checked and saw the printThrowable IS called if I make something get thrown in the test, and printThrowable IS being used to print the NPE in the failures, so those things do all work. Having the NPE when we don't have a connection to close is still misleading, so I think we should check them for null. If the connection failed with no Exception, it would hit a real NPE soon after which would be a useful message. But I think more likely the connect does throw an Exception, but in those builds/runs it didn't get captured in the log for some unclear reason. We should still fail correctly - if we are handling an Exception, even if our print was lost, we are throwing a RuntimeException, and with the finally block not throwing a new NPE, we should see that RuntimeException and have the test fail. 8-) ------------- PR: https://git.openjdk.org/jdk/pull/11881
