On Mon, 3 Nov 2025 16:40:05 GMT, Patricio Chilano Mateo <[email protected]> wrote:
>> Jorn Vernee has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update bug number in tests >> >> Co-authored-by: Chen Liang <[email protected]> > > src/hotspot/share/prims/scopedMemoryAccess.cpp line 94: > >> 92: >> 93: static bool is_accessing_session(JavaThread* jt, oop session, bool& >> in_scoped) { >> 94: if (jt->is_throwing_unsafe_access_error()) { > > If we assume arbitrary Java code in JVMTI callbacks this might return true > but the target could be in a different nested scoped access. I think we > should check we are in the no agent case before bailing out. My assumption is that once an unsafe access error is thrown, we don't expect execution to continue. I suppose it is technically possible to catch the exception, either in Java code or in the native agent code, and then try to continue execution, but the program would be in an undefined state at that point already. In other words, I don't think anyone should be continuing execution after this exception happens. Although, it doesn't seem like a bad idea to keep walking here as a fail safe. > src/hotspot/share/runtime/javaThread.hpp line 1364: > >> 1362: JavaThread* _thread; >> 1363: public: >> 1364: ThrowingUnsafeAccessError(JavaThread* thread) : _thread(thread) { > > If we assume arbitrary Java code in JVMTI callbacks this could be executed > recursively and `_throwing_unsafe_access_error` be set to false while we are > within the outer caller. Although it’s fine since we will do a full stack > walk in `is_accessing_session`, we should add a comment why this recursive > case is okay (or save the old value as with `UnlockFlagSaver`). That's a good point. Since this is otherwise unrelated code, I'll make it safe for the re-entrant case as well. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27919#discussion_r2487300422 PR Review Comment: https://git.openjdk.org/jdk/pull/27919#discussion_r2487303301
