On Wed, 14 May 2025 19:27:30 GMT, Alex Menkov <amen...@openjdk.org> wrote:
>> Actually debuggee.classByName() returns null if the class is not found, and >> callers typically do not check for this, which results in an NPE later on. >> debuggee.classByName() can also throw an exception if it discovers something >> like multiple matches for the class name. >> >> I'm not so sure the added error handling is of any real benefit here, and >> just adds to the already overdone error handling. Exceptions seem to >> properly result in a quick exit, and in fact it is the error handling that >> can result in not exiting quickly due to trying to sync with the debuggee. >> I'd like to request not having to add additional checks when calling >> debuggee.classByName() > >> I'm not so sure the added error handling is of any real benefit here, and >> just adds to the already overdone error handling. Exceptions seem to >> properly result in a quick exit, and in fact it is the error handling that >> can result in not exiting quickly due to trying to sync with the debuggee. > > My concern is about debugee process. I'm not sure all tests have a debugees > which exits shortly after the debugger disconnects. So we can get stale > debuggee processes on test failures After some private discussion, Alex and I have agreed that the classByName() calls are ok as-is. They would need both null checking and exception checking, but this is unnecessary because either condition results in an exception that will result in the test existing quickly and gracefully, and with a proper exception backtrace (I tested these bad classByName() results with all tests in this PR). Also, there are many pre-existing cases of tests either not checking for null or not checking for an exception. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25190#discussion_r2092074462
