The updated patch attached.
Now the test is passed with the suggested fix and failed without it.
Thanks,
Serguei
On 8/4/17 15:45, serguei.spit...@oracle.com wrote:
On 8/4/17 14:26, Daniel D. Daugherty wrote:
On 8/4/17 3:17 PM, serguei.spit...@oracle.com wrote:
The patch is attached.
It may need some tweaks though.
I was not able to make it fail yet.
I don't think the original test had "failure" detection.
You were just supposed to notice that a pending monitor
was listed under the wrong list.
Nothing is listed.
Strange thing is I do not see the monitor events fired.
I'm using 10 for testing.
Thanks,
Serguei
Dan
On 8/4/17 12:45, Daniel D. Daugherty wrote:
Thanks Serguei!
I happen to be doing a test run this weekend that includes most of the
JPDA stack of tests so I'll include the following in my experiment:
$ hg log -v -r tip
changeset: 12872:bb66cd7c61b1
tag: 8185164.patch
tag: qtip
tag: tip
user: dcubed
date: Fri Aug 04 13:41:29 2017 -0600
files: src/share/vm/runtime/objectMonitor.cpp
description:
imported patch 8185164.patch
That will get the product code changes a complete round of testing
on Solaris X64 at least... :-)
Great!
Thanks,
Serguei
Dan
On 8/4/17 1:31 PM, serguei.spit...@oracle.com wrote:
Dan,
Thank you for letting me know about this discussion.
I'll try to convert the attached test case to the JTreg format.
Thanks,
Serguei
On 8/4/17 11:16, Daniel D. Daugherty wrote:
Adding Serguei to this thread directly since he's back from
vacation!
On 7/31/17 10:14 PM, David Holmes wrote:
Hi Dan,
On 26/07/2017 11:52 PM, Daniel D. Daugherty wrote:
On 7/26/17 12:11 AM, David Holmes wrote:
On 26/07/2017 10:27 AM, Yasumasa Suenaga wrote:
Hi Dan,
I've added some analysis to the bug report
Thanks!
I tried to fix this issue in
JvmtiEnvBase::get_owned_monitors() at first.
But it is difficult because we cannot know pending monitor if
thread state is MONITOR_CONTENDED_ENTER when
get_owned_monitor() is called.
I need to look closer at this when I get back from vacation
next week.
Seems like you're back already. :-)
A pending monitor should not be reported as owned (unless the
spec says otherwise) and it seems odd to me to fix the current
problem by marking the monitor as pending earlier.
It's the updating of the _current_pending_monitor field that
allows JvmtiEnvBase::get_locked_objects_in_frame() to recognize
that the monitor observed in the frame is only pending and
is not owned.
I put a fairly detailed note in the bug yesterday, but you
should look at that when you're officially back!
Thanks for clarifying things. I also added a comment to the bug
report.
I think the fix is sound and prevents anyone from observing the
case where the monitor will be seen in the stack-frame, but has
not yet been set as the "pending monitor". As far as I can tell
it is only this case (GetOwnedMonitorInfo from the
contended-monitor event callback in the current thread) that
will be able to observe the change.
One scenario that I worry about here is that a
GetCurrentContendedMonitor()
call on a target thread will now be able to return a non-NULL
value for the
object, when GetThreadState() will be able to return something
other than
blocked (on monitor enter) for the thread.
I don't see anything in the JVM/TI spec that says such a scenario is
wrong; I'm only worried about whether we have any tests that
would catch
this slight change in behavior. In any case, one of these
operations has
to "happen first":
- thread is marked as blocked
- monitor is flagged as contended
Currently, they happen in the above order and the fix proposes to
change the order and I see no reason not to do it.
I would like the test attached to the bug to be converted into a
native
JTREG test that lives in hotspot/test/serviceability/jvmti. See the
following test as a possible example:
hotspot/test/serviceability/jvmti/GetNamedModule
for how to do this... I haven't done one of these new native JTREG
tests myself, but I believe Serguei has...
Dan
Thanks,
David
Dan
Thanks,
David
Did you run the jdk repo's com/sun/jdi tests with your fix?
I have not done yet.
I have a trip until 28 July JST. So I will run it after that.
Yasumasa
On 2017/07/26 7:05, Daniel D. Daugherty wrote:
On 7/24/17 8:40 PM, Yasumasa Suenaga wrote:
Hi all,
I tried to get owned monitors in MonitorContendedEnter
JVMTI event handler.
However GetOwnedMonitorInfo JVMTI function returns a
monitor which is
not yet owned.
I attached reproducer to JBS. Please read README.md.
I think GetOwnedMonitorInfo() should not return a pending
monitor.
I uploaded webrev. Could you review?
http://cr.openjdk.java.net/~ysuenaga/JDK-8185164/webrev.00/
I hope this fix is applied to 8u or later release.
I cannot access JPRT. So I need a sponsor.
Thanks for the bug report. It's nice to have a test case and
a proposed
fix all in the bug report! I've added some analysis to the
bug report
and we'll need to run this fix through Oracle's JPDA test
stack which
is not (yet) open.
Did you run the jdk repo's com/sun/jdi tests with your fix?
Dan
Thanks,
Yasumasa
diff -r f4315a059412 make/test/JtregNative.gmk
--- a/make/test/JtregNative.gmk Tue Aug 01 08:53:32 2017 -0700
+++ b/make/test/JtregNative.gmk Fri Aug 04 19:31:38 2017 -0700
@@ -61,6 +61,7 @@
$(HOTSPOT_TOPDIR)/test/runtime/noClassDefFoundMsg \
$(HOTSPOT_TOPDIR)/test/compiler/floatingpoint/ \
$(HOTSPOT_TOPDIR)/test/compiler/calls \
+ $(HOTSPOT_TOPDIR)/test/serviceability/jvmti/GetOwnedMonitorInfo \
$(HOTSPOT_TOPDIR)/test/serviceability/jvmti/GetNamedModule \
$(HOTSPOT_TOPDIR)/test/serviceability/jvmti/IsModifiableModule \
$(HOTSPOT_TOPDIR)/test/serviceability/jvmti/AddModuleReads \
@@ -92,6 +93,7 @@
ifeq ($(TOOLCHAIN_TYPE), solstudio)
BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_liboverflow := -lc
BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libSimpleClassFileLoadHook := -lc
+ BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libGetOwnedMonitorInfoTest := -lc
BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libGetNamedModuleTest := -lc
BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libIsModifiableModuleTest := -lc
BUILD_HOTSPOT_JTREG_LIBRARIES_LDFLAGS_libAddModuleReadsTest := -lc
diff -r f4315a059412 src/share/vm/runtime/objectMonitor.cpp
--- a/src/share/vm/runtime/objectMonitor.cpp Tue Aug 01 08:53:32 2017 -0700
+++ b/src/share/vm/runtime/objectMonitor.cpp Fri Aug 04 19:31:38 2017 -0700
@@ -307,6 +307,8 @@
{ // Change java thread status to indicate blocked on monitor enter.
JavaThreadBlockedOnMonitorEnterState jtbmes(jt, this);
+ Self->set_current_pending_monitor(this);
+
DTRACE_MONITOR_PROBE(contended__enter, this, object(), jt);
if (JvmtiExport::should_post_monitor_contended_enter()) {
JvmtiExport::post_monitor_contended_enter(jt, this);
@@ -321,8 +323,6 @@
OSThreadContendState osts(Self->osthread());
ThreadBlockInVM tbivm(jt);
- Self->set_current_pending_monitor(this);
-
// TODO-FIXME: change the following for(;;) loop to straight-line code.
for (;;) {
jt->set_suspend_equivalent();
diff -r f4315a059412
test/serviceability/jvmti/GetOwnedMonitorInfo/GetOwnedMonitorInfoTest.java
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++
b/test/serviceability/jvmti/GetOwnedMonitorInfo/GetOwnedMonitorInfoTest.java
Fri Aug 04 19:31:38 2017 -0700
@@ -0,0 +1,77 @@
+/*
+ * Copyright (c) 2017, 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 the JVMTI GetOwnedMonitorInfo API
+ * @modules jdk.jdi
+ * @compile GetOwnedMonitorInfoTest.java
+ * @run main/othervm/native -agentlib:GetOwnedMonitorInfoTest
GetOwnedMonitorInfoTest
+ */
+
+import java.io.PrintStream;
+
+public class GetOwnedMonitorInfoTest implements Runnable {
+
+ static {
+ try {
+ System.loadLibrary("GetOwnedMonitorInfoTest");
+ } catch (UnsatisfiedLinkError ule) {
+ System.err.println("Could not load GetOwnedMonitorInfoTest
library");
+ System.err.println("java.library.path: "
+ + System.getProperty("java.library.path"));
+ throw ule;
+ }
+ }
+
+ native static int check();
+
+ public void run() {
+ String name = Thread.currentThread().getName();
+ try {
+ synchronized (GetOwnedMonitorInfoTest.class) {
+ Thread.sleep(100);
+ System.out.println("Thread in sync section: " + name);
+ }
+ } catch (Exception e) {
+ e.printStackTrace();
+ }
+ }
+
+ public static void main(String[] args) throws Exception {
+ Thread t1 = new Thread(new GetOwnedMonitorInfoTest());
+ Thread t2 = new Thread(new GetOwnedMonitorInfoTest());
+
+ t1.start();
+ t2.start();
+
+ t1.join();
+ t2.join();
+
+ int status = check();
+ if (status != 0) {
+ throw new RuntimeException("FAILED status returned from the
agent");
+ }
+ }
+}
diff -r f4315a059412
test/serviceability/jvmti/GetOwnedMonitorInfo/libGetOwnedMonitorInfoTest.c
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++
b/test/serviceability/jvmti/GetOwnedMonitorInfo/libGetOwnedMonitorInfoTest.c
Fri Aug 04 19:31:38 2017 -0700
@@ -0,0 +1,179 @@
+/*
+ * Copyright (c) 2017, 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"
+#include "jni.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
+
+#define TranslateError(err) "JVMTI error"
+
+#define PASSED 0
+#define FAILED 2
+
+static jint status = PASSED;
+
+static jint Agent_Initialize(JavaVM *jvm, char *options, void *reserved);
+
+JNIEXPORT void JNICALL
+MonitorContendedEnter(jvmtiEnv *jvmti, JNIEnv *env, jthread thread, jobject
monitor) {
+ jvmtiThreadInfo threadInfo;
+ jint monitorCount;
+ jobject *ownedMonitors;
+
+ (*jvmti)->GetThreadInfo(jvmti, thread, &threadInfo);
+ (*jvmti)->GetOwnedMonitorInfo(jvmti, thread, &monitorCount,
&ownedMonitors);
+
+ printf("MonitorContendedEnter: %s owns %d monitor(s)\n",
+ threadInfo.name, monitorCount);
+
+ (*jvmti)->Deallocate(jvmti, (unsigned char *)ownedMonitors);
+ (*jvmti)->Deallocate(jvmti, (unsigned char *)threadInfo.name);
+
+ if (monitorCount != 0) {
+ status = FAILED;
+ }
+}
+
+JNIEXPORT void JNICALL
+MonitorContendedEntered(jvmtiEnv *jvmti, JNIEnv *env, jthread thread, jobject
monitor) {
+ jvmtiThreadInfo threadInfo;
+ jint monitorCount;
+ jobject *ownedMonitors;
+
+ (*jvmti)->GetThreadInfo(jvmti, thread, &threadInfo);
+ (*jvmti)->GetOwnedMonitorInfo(jvmti, thread, &monitorCount,
&ownedMonitors);
+
+ printf("MonitorContendedEntered: %s owns %d monitor(s)\n",
+ threadInfo.name, monitorCount);
+
+ (*jvmti)->Deallocate(jvmti, (unsigned char *)ownedMonitors);
+ (*jvmti)->Deallocate(jvmti, (unsigned char *)threadInfo.name);
+
+ if (monitorCount != 1) {
+ status = FAILED;
+ }
+}
+
+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_1_8;
+}
+
+static
+jint Agent_Initialize(JavaVM *jvm, char *options, void *reserved) {
+ jint res;
+ jvmtiError err;
+ jvmtiEnv *jvmti;
+ jvmtiCapabilities caps;
+ jvmtiEventCallbacks callbacks;
+
+ printf("Agent_OnLoad started\n");
+
+ res = JNI_ENV_PTR(jvm)->GetEnv(JNI_ENV_ARG(jvm, (void **) &jvmti),
+ JVMTI_VERSION_1);
+ if (res != JNI_OK || jvmti == NULL) {
+ printf(" Error: wrong result of a valid call to GetEnv!\n");
+ return JNI_ERR;
+ }
+
+ err = (*jvmti)->GetPotentialCapabilities(jvmti, &caps);
+ if (err != JVMTI_ERROR_NONE) {
+ printf("Agent_OnLoad: error in JVMTI GetPotentialCapabilities: %d\n",
err);
+ return JNI_ERR;
+ }
+
+ err = (*jvmti)->AddCapabilities(jvmti, &caps);
+ if (err != JVMTI_ERROR_NONE) {
+ printf("Agent_OnLoad: error in JVMTI AddCapabilities: %d\n", err);
+ }
+
+ err = (*jvmti)->GetCapabilities(jvmti, &caps);
+ if (err != JVMTI_ERROR_NONE) {
+ printf("Agent_OnLoad: error in JVMTI GetCapabilities: %d\n", err);
+ return JNI_ERR;
+ }
+
+ if (!caps.can_generate_monitor_events) {
+ printf("Warning: Monitor events are not implemented\n");
+ }
+ if (!caps.can_get_owned_monitor_info) {
+ printf("Warning: GetOwnedMonitorInfo is not implemented\n");
+ }
+
+ callbacks.MonitorContendedEnter = &MonitorContendedEnter;
+ callbacks.MonitorContendedEntered = &MonitorContendedEntered;
+
+ err = (*jvmti)->SetEventCallbacks(jvmti, &callbacks,
sizeof(jvmtiEventCallbacks));
+ if (err != JVMTI_ERROR_NONE) {
+ printf("Agent_OnLoad: error in JVMTI SetEventCallbacks: %d\n", err);
+ }
+
+ err = (*jvmti)->SetEventNotificationMode(jvmti, JVMTI_ENABLE,
+
JVMTI_EVENT_MONITOR_CONTENDED_ENTER, NULL);
+ if (err != JVMTI_ERROR_NONE) {
+ printf("Agent_OnLoad: error in JVMTI SetEventNotificationMode #1:
%d\n", err);
+ }
+ err = (*jvmti)->SetEventNotificationMode(jvmti, JVMTI_ENABLE,
+
JVMTI_EVENT_MONITOR_CONTENDED_ENTERED, NULL);
+ if (err != JVMTI_ERROR_NONE) {
+ printf("Agent_OnLoad: error in JVMTI SetEventNotificationMode #2:
%d\n", err);
+ }
+ printf("Agent_OnLoad finished\n");
+ return JNI_OK;
+}
+
+JNIEXPORT jint JNICALL
+Java_GetOwnedMonitorInfoTest_check(JNIEnv *env, jclass cls) {
+ return status;
+}
+
+#ifdef __cplusplus
+}
+#endif
