8187289: NotifyFramePop request is not cleared if JVMTI_EVENT_FRAME_POP is disabled

Reviewed-by: sspitsyn, cjplummer

src/hotspot/share/prims/jvmtiEventController.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -508,14 +508,19 @@ JvmtiEventControllerPrivate::recompute_thread_enabled(JvmtiThreadState *state) {
 
   julong was_any_env_enabled = state->thread_event_enable()->_event_enabled.get_bits();
   julong any_env_enabled = 0;
+  // JVMTI_EVENT_FRAME_POP can be disabled (in the case FRAME_POP_BIT is not set),
+  // but we need to set interp_only if some JvmtiEnvThreadState has frame pop set
+  // to clear the request
+  bool has_frame_pops = false;
 
   {
-    // This iteration will include JvmtiEnvThreadStates whoses environments
+    // This iteration will include JvmtiEnvThreadStates whose environments
     // have been disposed.  These JvmtiEnvThreadStates must not be filtered
     // as recompute must be called on them to disable their events,
     JvmtiEnvThreadStateIterator it(state);
     for (JvmtiEnvThreadState* ets = it.first(); ets != NULL; ets = it.next(ets)) {
       any_env_enabled |= recompute_env_thread_enabled(ets, state);
+      has_frame_pops |= ets->has_frame_pops();
     }
   }
 
@@ -523,23 +528,23 @@ JvmtiEventControllerPrivate::recompute_thread_enabled(JvmtiThreadState *state) {
     // mark if event is truly enabled on this thread in any environment
     state->thread_event_enable()->_event_enabled.set_bits(any_env_enabled);
 
-    // compute interp_only mode
-    bool should_be_interp = (any_env_enabled & INTERP_EVENT_BITS) != 0;
-    bool is_now_interp = state->is_interp_only_mode();
-
-    if (should_be_interp != is_now_interp) {
-      if (should_be_interp) {
-        enter_interp_only_mode(state);
-      } else {
-        leave_interp_only_mode(state);
-      }
-    }
-
     // update the JavaThread cached value for thread-specific should_post_on_exceptions value
     bool should_post_on_exceptions = (any_env_enabled & SHOULD_POST_ON_EXCEPTIONS_BITS) != 0;
     state->set_should_post_on_exceptions(should_post_on_exceptions);
   }
 
+  // compute interp_only mode
+  bool should_be_interp = (any_env_enabled & INTERP_EVENT_BITS) != 0 || has_frame_pops;
+  bool is_now_interp = state->is_interp_only_mode();
+
+  if (should_be_interp != is_now_interp) {
+    if (should_be_interp) {
+      enter_interp_only_mode(state);
+    } else {
+      leave_interp_only_mode(state);
+    }
+  }
+
   return any_env_enabled;
 }
 

src/hotspot/share/prims/jvmtiExport.cpp

@@ -1576,9 +1576,9 @@ void JvmtiExport::post_method_exit(JavaThread *thread, Method* method, frame cur
     }
   }
 
-  if (state->is_enabled(JVMTI_EVENT_FRAME_POP)) {
+  JvmtiEnvThreadStateIterator it(state);
-    JvmtiEnvThreadStateIterator it(state);
+  for (JvmtiEnvThreadState* ets = it.first(); ets != NULL; ets = it.next(ets)) {
-    for (JvmtiEnvThreadState* ets = it.first(); ets != NULL; ets = it.next(ets)) {
+    if (ets->has_frame_pops()) {
       int cur_frame_number = state->cur_stack_depth();
 
       if (ets->is_frame_pop(cur_frame_number)) {

test/hotspot/jtreg/serviceability/jvmti/NotifyFramePop/NotifyFramePopTest.java

@@ -0,0 +1,97 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @summary Verifies NotifyFramePop request is cleared if JVMTI_EVENT_FRAME_POP is disabled
+ * @compile NotifyFramePopTest.java
+ * @run main/othervm/native -agentlib:NotifyFramePopTest NotifyFramePopTest
+ */
+
+public class NotifyFramePopTest {
+    static {
+        try {
+            System.loadLibrary("NotifyFramePopTest");
+        } catch (UnsatisfiedLinkError ex) {
+            System.err.println("Could not load NotifyFramePopTest library");
+            System.err.println("java.library.path:"
+                + System.getProperty("java.library.path"));
+            throw ex;
+        }
+    }
+
+    public static void main(String args[]) {
+        if (!canGenerateFramePopEvents()) {
+            log("FramePop event is not supported - skipping the test");
+            return;
+        }
+
+        // Sanity testing that FRAME_POP works.
+        test("sanity", true, () -> {
+            setFramePopNotificationMode(true);
+            notifyFramePop(null);
+        });
+
+        // Request notification and then disable FRAME_POP event notification.
+        // This should not prevent the notification for the frame being cleared
+        // when we return from the method.
+        test("requestAndDisable", false, () -> {
+            setFramePopNotificationMode(true);
+            notifyFramePop(null);
+            setFramePopNotificationMode(false);
+        });
+
+        // Ensure there is no pending event
+        test("ensureCleared", false, () -> {
+            setFramePopNotificationMode(true);
+        });
+
+        log("Test PASSED");
+    }
+
+    private native static boolean canGenerateFramePopEvents();
+    private native static void setFramePopNotificationMode(boolean enabled);
+    private native static void notifyFramePop(Thread thread);
+    private native static boolean framePopReceived();
+
+    private static void log(String msg) {
+        System.out.println(msg);
+    }
+
+    private interface Test {
+        void test();
+    }
+
+    private static void test(String name, boolean framePopExpected, Test theTest) {
+        log("test: " + name);
+        theTest.test();
+        boolean actual = framePopReceived();
+        if (framePopExpected != actual) {
+            throw new RuntimeException("unexpected notification:"
+                    + " FramePop expected: " + (framePopExpected ? "yes" : "no")
+                    + ", actually received: " + (actual ? "yes" : "no"));
+        }
+        log("  - OK (" + (actual ? "received" : "NOT received") + ")");
+    }
+
+}

test/hotspot/jtreg/serviceability/jvmti/NotifyFramePop/libNotifyFramePopTest.c

@@ -0,0 +1,173 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include "jvmti.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#ifndef JNI_ENV_ARG
+
+#ifdef __cplusplus
+#define JNI_ENV_ARG(x, y) y
+#define JNI_ENV_PTR(x) x
+#else
+#define JNI_ENV_ARG(x,y) x, y
+#define JNI_ENV_PTR(x) (*x)
+#endif
+
+#endif
+
+static jvmtiEnv *jvmti = NULL;
+static jvmtiCapabilities caps;
+static jvmtiEventCallbacks callbacks;
+static jboolean framePopReceived = JNI_FALSE;
+
+static jint Agent_Initialize(JavaVM *jvm, char *options, void *reserved);
+
+JNIEXPORT
+jint JNICALL Agent_OnLoad(JavaVM *jvm, char *options, void *reserved) {
+    return Agent_Initialize(jvm, options, reserved);
+}
+
+JNIEXPORT
+jint JNICALL Agent_OnAttach(JavaVM *jvm, char *options, void *reserved) {
+    return Agent_Initialize(jvm, options, reserved);
+}
+
+JNIEXPORT
+jint JNICALL JNI_OnLoad(JavaVM *jvm, void *reserved) {
+    return JNI_VERSION_9;
+}
+
+
+static void reportError(const char *msg, int err) {
+    fprintf(stdout, "%s, error: %d\n", msg, err);
+    fflush(stdout);
+}
+
+
+static void JNICALL
+FramePop(jvmtiEnv *jvmti_env, JNIEnv *env, jthread thread,
+         jmethodID method, jboolean wasPoppedByException)
+{
+    jvmtiError err;
+    jclass cls = NULL;
+    char* csig = NULL;
+    char* name = NULL;
+    char* sign = NULL;
+
+    framePopReceived = JNI_TRUE;
+
+    err = (*jvmti_env)->GetMethodDeclaringClass(jvmti_env, method, &cls);
+    if (err != JVMTI_ERROR_NONE) {
+        reportError("FramePop: GetMethodDeclaringClass failed", err);
+    }
+    err = (*jvmti_env)->GetClassSignature(jvmti_env, cls, &csig, NULL);
+    if (err != JVMTI_ERROR_NONE) {
+        reportError("FramePop: GetClassSignature failed", err);
+    }
+    err = (*jvmti_env)->GetMethodName(jvmti_env, method, &name, &sign, NULL);
+    if (err != JVMTI_ERROR_NONE) {
+        reportError("FramePop: GetMethodName failed", err);
+    }
+    fprintf(stdout, "FramePop event from method: %s %s%s\n", csig, name, sign);
+    fflush(stdout);
+
+    (*jvmti_env)->Deallocate(jvmti_env, (unsigned char*)csig);
+    (*jvmti_env)->Deallocate(jvmti_env, (unsigned char*)name);
+    (*jvmti_env)->Deallocate(jvmti_env, (unsigned char*)sign);
+}
+
+static
+jint Agent_Initialize(JavaVM *jvm, char *options, void *reserved) {
+    jint res;
+    jvmtiError err;
+
+    res = JNI_ENV_PTR(jvm)->GetEnv(JNI_ENV_ARG(jvm, (void **) &jvmti), JVMTI_VERSION_9);
+    if (res != JNI_OK || jvmti == NULL) {
+        reportError("GetEnv(JVMTI_VERSION_9) failed", res);
+        return JNI_ERR;
+    }
+    err = (*jvmti)->GetPotentialCapabilities(jvmti, &caps);
+    if (err != JVMTI_ERROR_NONE) {
+        reportError("GetPotentialCapabilities failed", err);
+        return JNI_ERR;
+    }
+    err = (*jvmti)->AddCapabilities(jvmti, &caps);
+    if (err != JVMTI_ERROR_NONE) {
+        reportError("AddCapabilities failed",err);
+        return JNI_ERR;
+    }
+    err = (*jvmti)->GetCapabilities(jvmti, &caps);
+    if (err != JVMTI_ERROR_NONE) {
+        reportError("GetCapabilities failed", err);
+        return JNI_ERR;
+    }
+    if (caps.can_generate_frame_pop_events) {
+        callbacks.FramePop = &FramePop;
+        err = (*jvmti)->SetEventCallbacks(jvmti, &callbacks, sizeof(callbacks));
+        if (err != JVMTI_ERROR_NONE) {
+            reportError("SetEventCallbacks failed", err);
+            return JNI_ERR;
+        }
+    }
+    return JNI_OK;
+}
+
+JNIEXPORT jboolean JNICALL
+Java_NotifyFramePopTest_canGenerateFramePopEvents(JNIEnv *env, jclass cls) {
+    return caps.can_generate_frame_pop_events ? JNI_TRUE : JNI_FALSE;
+}
+
+JNIEXPORT void JNICALL
+Java_NotifyFramePopTest_setFramePopNotificationMode(JNIEnv *env, jclass cl, jboolean enable) {
+    jvmtiEventMode mode = enable ? JVMTI_ENABLE : JVMTI_DISABLE;
+    jvmtiError err = (*jvmti)->SetEventNotificationMode(jvmti, mode, JVMTI_EVENT_FRAME_POP, NULL);
+    if (err != JVMTI_ERROR_NONE) {
+        reportError("Failed to set notification mode for FRAME_POP events", err);
+    }
+}
+
+JNIEXPORT void JNICALL
+Java_NotifyFramePopTest_notifyFramePop(JNIEnv *env, jclass cls, jthread thread)
+{
+    jvmtiError err= (*jvmti)->NotifyFramePop(jvmti, thread, 1);
+    if (err != JVMTI_ERROR_NONE) {
+        reportError("NotifyFramePop failed", err);
+    }
+}
+
+JNIEXPORT jboolean JNICALL
+Java_NotifyFramePopTest_framePopReceived(JNIEnv *env, jclass cls) {
+    jboolean result = framePopReceived;
+    framePopReceived = JNI_FALSE;
+    return result;
+}
+
+#ifdef __cplusplus
+}
+#endif