JDK-8251118 : BiasedLocking::preserve_marks should not have a HandleMark
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 16
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2020-08-04
  • Updated: 2020-09-11
  • Resolved: 2020-08-13
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 11 JDK 16
11.0.10Fixed 16 b12Fixed
Related Reports
Relates :  
JDK-8249192 - MonitorInfo stores raw oops across safepoints
Description
Recent changes added a HandleMark and this function creates Handles to hold oops over GC.  It breaks the usual scoping conventions.  Since BiasedLocking is default false, we didn't find this in testing.  But Thanks to [~hseigel] for finding it.

To reproduce change this test and run.

diff --git a/test/hotspot/jtreg/runtime/7158988/FieldMonitor.java b/test/hotspot/jtreg/runtime/7158988/FieldMonitor.java
--- a/test/hotspot/jtreg/runtime/7158988/FieldMonitor.java
+++ b/test/hotspot/jtreg/runtime/7158988/FieldMonitor.java
@@ -62,7 +62,7 @@
 
   public static final String CLASS_NAME = "TestPostFieldModification";
   public static final String FIELD_NAME = "value";
-  public static final String ARGUMENTS = "-Xshare:off -Xlog:gc";
+  public static final String ARGUMENTS = "-Xshare:off -Xlog:gc -XX:+UseBiasedLocking";
 
   public static void main(String[] args)
       throws IOException, InterruptedException {
Comments
Fix Request (15u) Followup after JDK-8249192 backport. Patch applies cleanly to 15u. Passes tier{1,2} with and without Shenandoah (that seems to trigger this bug).
09-09-2020
Fix Request (11u) Followup after JDK-8249192 backport. Patch applies with minor differences in test. 11u RFR [acked by goetz]: https://mail.openjdk.java.net/pipermail/jdk-updates-dev/2020-September/003733.html. Passes tier{1,2,3} with and without Shenandoah (that seems to trigger this bug).
02-09-2020
[~shade] The code change is in 11.0.10-oracle but got there via a different path.
31-08-2020
I stepped on this when testing JDK-8249192 11u backport with Shenandoah (which keeps biased locking enabled, and has aggressive tests for Full GC path that uses {preserve,restore}_marks). The patch seems to solve the issue. But I do wonder: if JDK-8249192 is in 11.0.10-oracle, shouldn't this one be in 11.0.10-oracle as well?
31-08-2020
URL: https://hg.openjdk.java.net/jdk/jdk/rev/abb125b87e74 User: pchilanomate Date: 2020-08-13 15:43:45 +0000
13-08-2020
Thanks [~pchilanomate] - that worked for me!
06-08-2020
[~dholmes] I think the fix is to remove the added HandleMark from BiasedLocking::preserve_marks(). I run test FieldMonitor.java with that fix and it didn't failed. But I haven't run the fix in mach5 yet or done other testing to confirm.
06-08-2020
I've also hit the: # SIGSEGV (0xb) at pc=0x0000ffffbbdce250, pid=11997, tid=12006 # Problematic frame: # V [libjvm.so+0x4be250] BiasedLocking::restore_marks()+0xf8 in local testing of another issue. Is there a quick and dirty fix for this crash?
06-08-2020
Also in the patch for JDK-8249192, vframe.hpp shouldn't include handles.inline.hpp (only cpp and inline.hpp files can include inline.hpp). It doesn't look like it needs handles.inline.hpp, only handles.hpp.
05-08-2020
The methods are called only by the VM thread afaics. I may have not search that far up in the VMThread or simply missed that HandleMark. Seeing it now, vmThread.cpp:525.
05-08-2020
Hi Thomas, is preserve_marks() called by the VMThread, by GC workers or both? If we remove the HandleMark then it will work okay for the VMThread since there is a HandleMark before executing the VM operation, but not sure about GC threads. In any case that applies also before JDK-8249192, so we were either already cleaning up the handles or not.
05-08-2020
> Can we add a variant of that test that explicitly enables biased locking? Yes.
05-08-2020
The _preserved_oop_stack is later used and cleaned out in BiasedLocking::restore_marks(). So the intermittent HandleMark makes the handles invalid across the preserve_marks/restore_marks range. I.e. after the preserve_marks() call there can be (actually likely is) a GC..., but that HandleMark scope closes before that. :( Since all the code using BiasedLocking::preserve_marks()/restore_marks() is scoped in a safepoint, the fix is to remove that additional HandleMark afaiu. Briefly looking through the callers of BiasedLocking::preserve_marks(), not all seem to provide an obvious HandleMark in the preserve_marks/restore_marks scope. Can we add a variant of that test that explicitly enables biased locking?
05-08-2020
_preserved_mark_stack = new (ResourceObj::C_HEAP, mtGC) GrowableArray<markWord>(10, mtGC); _preserved_oop_stack = new (ResourceObj::C_HEAP, mtGC) GrowableArray<Handle>(10, mtGC); Thread* cur = Thread::current(); ResourceMark rm(cur); HandleMark hm(cur); In the biasedLocking.cpp code above _preserved_oop_stack pushes Handles. I can't find what clears the handles from the Thread though.
04-08-2020