On Tue, 16 May 2023 20:21:39 GMT, Patricio Chilano Mateo <pchilanom...@openjdk.org> wrote:
>> Summary: Thumbs up. This is most definitely NOT a trivial fix. >> >> Normally the way I like to review these kinds of fixes is to map the failure >> modes back to the fix just to make sure I understand how each of the >> failure modes is covered by the changes. For this particular bug that has >> NOT been an easy thing to do. I believe the failure modes are complicated >> to follow because we "lost" the synchronization of being in the >> `VTMS_unmount_begin` transition which allowed a thread calling >> `recompute_enabled()` to race with our ending/exiting vthread/cthread >> combo. Ouch. >> >> The fix does restore the synchronization of being in the >> `VTMS_unmount_begin` transition so I can see how we would no longer >> be racing with the `recompute_enabled()` calling thread. > >> Summary: Thumbs up. This is most definitely NOT a trivial fix. >> >> Normally the way I like to review these kinds of fixes is to map the failure >> modes back to the fix just to make sure I understand how each of the failure >> modes is covered by the changes. For this particular bug that has NOT been >> an easy thing to do. I believe the failure modes are complicated to follow >> because we "lost" the synchronization of being in the `VTMS_unmount_begin` >> transition which allowed a thread calling `recompute_enabled()` to race with >> our ending/exiting vthread/cthread combo. Ouch. >> > Exactly. In the reproducer I aimed for the ThreadsListHandle crash, but while > working on it I also hit the other ones. > @pchilano - This bug has had reported sightings in Tier[34568] so Tier[1-3] > testing is probably not enough. However, since you've been able to create a > tight reproducer, your testing might be sufficient. Of course, as > @dholmes-ora likes to say: only time will tell! > I actually also tested it in mach5 tiers 4-6 after the initial post. : ) ------------- PR Comment: https://git.openjdk.org/jdk/pull/13949#issuecomment-1550309385
