[10] RFR: 8185164: GetOwnedMonitorInfo() returns incorrect owned monitor
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Fri Aug 4 21:17:46 UTC 2017
The patch is attached.
It may need some tweaks though.
I was not able to make it fail yet.
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.spitsyn at 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
>>>>>>>>
>>>>>
>>>
>>
>
-------------- next part --------------
cat GetOwnedMonitorInfoTest.patch
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 14:11:05 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 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 14:11:05 2017 -0700
@@ -0,0 +1,76 @@
+/*
+ * 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(){
+ try {
+ synchronized (GetOwnedMonitorInfoTest.class) {
+ Thread.sleep(1000);
+ System.out.println("Exit " + Thread.currentThread().getName());
+ }
+ } 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("Non-zero status returned from the agent: " + status);
+ }
+ }
+}
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 14:11:05 2017 -0700
@@ -0,0 +1,141 @@
+/*
+ * 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 result = PASSED;
+static jint contendedEnterMonitorCount = 0;
+
+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);
+ contendedEnterMonitorCount = monitorCount;
+}
+
+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);
+
+ result = (contendedEnterMonitorCount == monitorCount + 1) ? PASSED : 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;
+ jvmtiEnv *jvmti;
+ jvmtiCapabilities caps;
+ jvmtiEventCallbacks callbacks;
+
+ res = JNI_ENV_PTR(jvm)->GetEnv(JNI_ENV_ARG(jvm, (void **) &jvmti),
+ JVMTI_VERSION_9);
+ if (res != JNI_OK || jvmti == NULL) {
+ printf(" Error: wrong result of a valid call to GetEnv!\n");
+ return JNI_ERR;
+ }
+
+ caps.can_generate_monitor_events = 1;
+ caps.can_get_owned_monitor_info = 1;
+ (*jvmti)->AddCapabilities(jvmti, &caps);
+
+ callbacks.MonitorContendedEnter = &MonitorContendedEnter;
+ callbacks.MonitorContendedEntered = &MonitorContendedEntered;
+ (*jvmti)->SetEventCallbacks(jvmti, &callbacks, sizeof(jvmtiEventCallbacks));
+
+ (*jvmti)->SetEventNotificationMode(jvmti, JVMTI_ENABLE,
+ JVMTI_EVENT_MONITOR_CONTENDED_ENTER, NULL);
+ (*jvmti)->SetEventNotificationMode(jvmti, JVMTI_ENABLE,
+ JVMTI_EVENT_MONITOR_CONTENDED_ENTERED, NULL);
+ return JNI_OK;
+}
+
+JNIEXPORT jint JNICALL
+Java_GetOwnedMonitorInfoTest_check(JNIEnv *env, jclass cls) {
+ return result;
+}
+
+#ifdef __cplusplus
+}
+#endif
More information about the serviceability-dev
mailing list