On Thu, 21 Oct 2021 15:05:19 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>>> move the locking and unlocking into doPendingTasks(). At the start of the >>> node->handlingAppResume block, grab handlerLock and threadLock, and explain >>> that handlerLock is being grabbed because trackAppResume() will (indirectly) >>> try to grab it, and grabbing it before threadLock is necessary for lock >>> ordering. Then grab threadLock and hold onto it until the end of the block. >> >> So threadLock would be released at the end of the new block that depends on >> node->handlingAppResume. After that block follow accesses to >> `pendingInterrupt` >> and `pendingStop` that would be unsynchronized then. I think they do need to >> be >> synchronized through threadLock though. > > If you are worried about another thread changing those fields after they have > already been checked, then you can use the threadLock around them also. So > you can grab threadLock before the `if (node->handlingAppResume)` and release > after the `pendingInterrupt` and `pendingStop` references. I don't think it's > really needed, because this is the only place where the flags are set false > (and some action is taken when true), but there is no harm in holding the > threadLock here, and it doesn't make the locking any more complicated. I have hoisted the locking from `trackAppResume()` up to `doPendingTasks()` where it is more visible. The change in that form is sufficient to fix the deadlock issues. Would it be ok for you to do further enhancements in a follow-up RFE? ------------- PR: https://git.openjdk.java.net/jdk/pull/5849
