RFR: 8356811: Some nsk/jdi tests can fetch ThreadReference from static field in the debuggee: part 4 [v2]
Chris Plummer
cjplummer at openjdk.org
Thu May 15 23:00:51 UTC 2025
On Wed, 14 May 2025 19:27:30 GMT, Alex Menkov <amenkov at 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
More information about the serviceability-dev
mailing list