> The following patch fixes a bug introduced while refactoring the 
> VirtualThreadStart/End events. Specifically, the code to delete the 
> JvmtiThreadState of a terminating vthread was moved before we start the VTMS 
> transition. That allowed said code to run concurrently with 
> recompute_enabled() leading to different crashing modes. I wrote the detailed 
> sequence of events leading to the crash in the bug comments. 
> 
> To fix it I moved the cleanup code back after the call to 
> VTMS_unmount_begin(). Now, since the rebinding of the JvmtiThreadState to 
> that of the carrier has to be done after this cleanup code is executed, 
> otherwise we would delete the wrong JvmtiThreadState state, I had to add a 
> boolean argument to VTMS_unmount_begin() to differentiate the last unmount 
> call from the other ones. This is unfortunate since ideally 
> VTMS_unmount_begin() would be oblivious to these two cases as with 
> VTMS_mount_end() where we don't need to check if this is the first mount.
> I looked for other ways to solve it instead of the extra boolean argument but 
> wasn't convinced. One way would be to have another 
> JvmtiExport::cleanup_thread() that would handle this case. Another way which 
> is very simple is to move the rebind_to_jvmti_thread_state_of() call to 
> VTMS_unmount_end() instead. But that means during the transition the 
> _jvmti_thread_state field of the carrier would be either null or that of the 
> vthread, unlike today which is always that of the carrier during the 
> transitions. I didn't want to change that behavior in this fix but I can also 
> explore that route.
> 
> I tested the patch with the reproducer I attached to the bug, plus I also run 
> tiers1-3 in mach5.
> 
> Thanks,
> Patricio

Patricio Chilano Mateo has updated the pull request incrementally with one 
additional commit since the last revision:

  added new test

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/13949/files
  - new: https://git.openjdk.org/jdk/pull/13949/files/f22cc954..8a60ce1d

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=13949&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=13949&range=01-02

  Stats: 210 lines in 2 files changed: 210 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/13949.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/13949/head:pull/13949

PR: https://git.openjdk.org/jdk/pull/13949

Reply via email to