From 5d34ddbacba99a104cc882f3dec2132566c67b3c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Erik=20=C3=96sterlund?= <eosterlund@openjdk.org>
Date: Thu, 23 Nov 2023 16:22:18 +0000
Subject: [PATCH] Comments from Jorn

---
 .../share/prims/scopedMemoryAccess.cpp        | 47 ++-----------------
 .../internal/foreign/MemorySessionImpl.java   |  2 +-
 .../jdk/internal/foreign/SharedSession.java   |  2 +-
 .../misc/X-ScopedMemoryAccess.java.template   |  4 +-
 4 files changed, 8 insertions(+), 47 deletions(-)

diff --git a/src/hotspot/share/prims/scopedMemoryAccess.cpp b/src/hotspot/share/prims/scopedMemoryAccess.cpp
index eac9af32b88f8..62546c4fe51f6 100644
--- a/src/hotspot/share/prims/scopedMemoryAccess.cpp
+++ b/src/hotspot/share/prims/scopedMemoryAccess.cpp
@@ -35,38 +35,6 @@
 #include "runtime/sharedRuntime.hpp"
 #include "runtime/vframe.inline.hpp"
 
-class CloseScopedMemoryFindOopClosure : public OopClosure {
-  oop _deopt;
-  bool _found;
-
-public:
-  CloseScopedMemoryFindOopClosure(jobject deopt) :
-      _deopt(JNIHandles::resolve(deopt)),
-      _found(false) {}
-
-  template <typename T>
-  void do_oop_work(T* p) {
-    if (_found) {
-      return;
-    }
-    if (RawAccess<>::oop_load(p) == _deopt) {
-      _found = true;
-    }
-  }
-
-  virtual void do_oop(oop* p) {
-    do_oop_work(p);
-  }
-
-  virtual void do_oop(narrowOop* p) {
-    do_oop_work(p);
-  }
-
-  bool found() {
-    return _found;
-  }
-};
-
 static bool is_in_scoped_access(JavaThread* jt, oop session) {
   const int max_critical_stack_depth = 10;
   int depth = 0;
@@ -147,17 +115,10 @@ class CloseScopedMemoryClosure : public HandshakeClosure {
     }
 
     ResourceMark rm;
-    if (_session != nullptr && last_frame.is_compiled_frame() && last_frame.can_be_deoptimized()) {
-      CloseScopedMemoryFindOopClosure cl(_session);
-      CompiledMethod* cm = last_frame.cb()->as_compiled_method();
-
-      /* FIXME: this doesn't work if reachability fences are violated by C2
-      last_frame.oops_do(&cl, nullptr, &register_map);
-      if (cl.found()) {
-           //Found the deopt oop in a compiled method; deoptimize.
-           Deoptimization::deoptimize(jt, last_frame);
-      }
-      so... we unconditionally deoptimize, for now: */
+    if (last_frame.is_compiled_frame() && last_frame.can_be_deoptimized()) {
+      // FIXME: we would like to conditionally deoptimize only if the corresponding
+      // _session is reachable from the frame, but reachabilityFence doesn't currently
+      // work the way it should. Therefore we deopt unconditionally for now.
       Deoptimization::deoptimize(jt, last_frame);
     }
 
diff --git a/src/java.base/share/classes/jdk/internal/foreign/MemorySessionImpl.java b/src/java.base/share/classes/jdk/internal/foreign/MemorySessionImpl.java
index 269d22705d88a..aef62be815160 100644
--- a/src/java.base/share/classes/jdk/internal/foreign/MemorySessionImpl.java
+++ b/src/java.base/share/classes/jdk/internal/foreign/MemorySessionImpl.java
@@ -60,7 +60,7 @@ public abstract sealed class MemorySessionImpl
     static final VarHandle STATE;
     static final int MAX_FORKS = Integer.MAX_VALUE;
 
-    public static final ScopedMemoryAccess.ScopedAccessError ALREADY_CLOSED = new ScopedMemoryAccess.ScopedAccessError(MemorySessionImpl::alreadyClosed);
+    static final ScopedMemoryAccess.ScopedAccessError ALREADY_CLOSED = new ScopedMemoryAccess.ScopedAccessError(MemorySessionImpl::alreadyClosed);
     static final ScopedMemoryAccess.ScopedAccessError WRONG_THREAD = new ScopedMemoryAccess.ScopedAccessError(MemorySessionImpl::wrongThread);
     // This is the session of all zero-length memory segments
     public static final MemorySessionImpl GLOBAL_SESSION = new GlobalSession();
diff --git a/src/java.base/share/classes/jdk/internal/foreign/SharedSession.java b/src/java.base/share/classes/jdk/internal/foreign/SharedSession.java
index 37c351e9bf8cf..1569589ef9b81 100644
--- a/src/java.base/share/classes/jdk/internal/foreign/SharedSession.java
+++ b/src/java.base/share/classes/jdk/internal/foreign/SharedSession.java
@@ -83,7 +83,7 @@ void justClose() {
         } else if (prevState != OPEN) {
             throw alreadyAcquired(prevState);
         }
-        SCOPED_MEMORY_ACCESS.closeScope(this);
+        SCOPED_MEMORY_ACCESS.closeScope(this, ALREADY_CLOSED);
     }
 
     /**
diff --git a/src/java.base/share/classes/jdk/internal/misc/X-ScopedMemoryAccess.java.template b/src/java.base/share/classes/jdk/internal/misc/X-ScopedMemoryAccess.java.template
index 39910c160ece4..0147b3bacd86e 100644
--- a/src/java.base/share/classes/jdk/internal/misc/X-ScopedMemoryAccess.java.template
+++ b/src/java.base/share/classes/jdk/internal/misc/X-ScopedMemoryAccess.java.template
@@ -83,8 +83,8 @@ public class ScopedMemoryAccess {
         registerNatives();
     }
 
-    public void closeScope(MemorySessionImpl session) {
-        closeScope0(session, MemorySessionImpl.ALREADY_CLOSED);
+    public void closeScope(MemorySessionImpl session, ScopedAccessError error) {
+        closeScope0(session, error);
     }
 
     native void closeScope0(MemorySessionImpl session, ScopedAccessError error);