On Fri, 15 Oct 2021 09:17:12 GMT, Richard Reingruber <rr...@openjdk.org> wrote:

>> Ok. So you just need to reacquire the threadLock before the `findThread()` 
>> call and before exiting the while loop, and hold it until after the 
>> `trackAppResume()` call. I guess it ok then. But this exiting of the 
>> handlerLock here and in `blockOnDebuggerSuspend()` is always going to arouse 
>> suspicion for anyone that reads the code. The first question will be if the 
>> lock does not need to be held here, why grab it in the first place? We know 
>> the answer is this lock ordering issue that turns up when `trackAppResume()` 
>> is later called, but this will be far from obvious to the reader. It might 
>> be possible to make this a bit clearer with some code restructuring, like 
>> maybe combining `blockOnDebuggerSuspend()` and `trackAppResume()` into one 
>> function (I'm not sure that this will actually help, but maybe something to 
>> consider), but at the very least we need some comments calling out why 
>> handlerLock is held in the first place and why it is ok to release it.
>
> I've added the comments. Maybe I should really combine 
> `blockOnDebuggerSuspend()` and `trackAppResume()` into one function...

Hm, I think this can be simplified by swaping blockOnDebuggerSuspend() and 
trackAppResume(). Can't try it today but will on Monday.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5849

Reply via email to