On Thu, 24 Apr 2025 15:34:40 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
>> nsk.share.jdi.Debugee.waitingEvent() takes a timeout argument. However, if >> the call to EventQueue.remove() times out and returns null, waitingEvent() >> just keep on retrying. The logic is incorrect for a null result. It should >> immediately throw an exception. The retry path is only meant for filtering >> out other events, and it updates "timeLeft' before retrying to avoid >> endlessly repeating the loop. >> >> Tested by running all of nsk/jdi locally on linux-x64-debug > > Marked as reviewed by lmesnik (Reviewer). > @lmesnik @alexmenkov I just noticed an inconsistency with my fix. If the code > falls out of the while loop, it returns null, whereas the fix I added throws > an exception if eventQueue.remove() times out. Initially I had fixed it by > returning null here also, but the result of returning null meant an NPE in > the caller in the one case I had where this code was timing out. However, > that was new code I'm experimenting with that added a waitingEvent() call and > didn't check the result, so it got an NPE. It seems that all other callers do > check the result (via instanceof), so they would not result in an NPE. So > perhaps the better choice here is to return null instead of throwing an > exception. Is there a need to correct the PR description to reflect the updated approach? ------------- PR Comment: https://git.openjdk.org/jdk/pull/24839#issuecomment-2829252325
