[10] RFR: 8185164: GetOwnedMonitorInfo() returns incorrect owned monitor
Yasumasa Suenaga
yasuenag at gmail.com
Mon Aug 7 05:10:03 UTC 2017
Hi David,
>>> Style query: do we do indent of 2 or 4 in hotspot C code tests ?
>>
>>
>> Fixed in C code.
>
>
> I was asking a question - I'm not sure if we use 2 or 4. Now looking at some
> other JVMTI tests they seem to use 4. I didn't do an exhaustive check to see
> if there is a predominant style.
IMHO we should use 2 for indent because it uses for HotSpot and this
is testcase for HotSpot.
However, I'm not sure.
I will upload new webrev after it is clarified.
Thanks,
Yasumasa
2017-08-07 13:50 GMT+09:00 David Holmes <david.holmes at oracle.com>:
> Hi Yasumasa,
>
> On 7/08/2017 2:02 PM, Yasumasa Suenaga wrote:
>>
>> Hi David,
>>
>> I uploaded new webrev:
>>
>> http://cr.openjdk.java.net/~ysuenaga/JDK-8185164/webrev.04/
>
>
> Thanks for taking up all the suggestions! A few follow ups below ...
>
>>> I have one concern with the test. Are you certain that the only possible
>>> contended-enter event can come from the attempt to lock the
>>> GetOwnedMonitorInfoTest.class object? Something from -Xcomp perhaps? The
>>> test would likely still pass, but it would be checking the "wrong" event.
>>
>>
>> I guess you mean other place (e.g. in class library) might generate
>> MonitorContendedEnter event.
>> So I check the class of monitor object in JNI test module.
>
>
> Thanks for that! The change to using lambda may result in unexpected locking
> in itself. :)
>
>>> 27 * @summary Verifies the JVMTI GetOwnedMonitorInfo API
>>>
>>> That's a bit of an overstatement - this only verifies that a contended
>>> monitor does not show up in the set of owned monitors. (Even that is
>>> somewhat inaccurate)
>>
>>
>> I modified the summary, but I'm not good at English.
>> So please tell me if it is incorrect :-)
>
>
> I would prefer simply:
>
> @summary Checks that a contended monitor does not show up in the list of
> owned monitors
>
>>> Style-nit:
>>>
>>> 35 public class GetOwnedMonitorInfoTest implements Runnable {
>>>
>>> There is no need to implement Runnable and provide the run() method as an
>>> instance method of the class because there is no state. The simpler
>>> construct would be to define an anonymous inner class when creating the
>>> thread:
>>> ...
>>
>>
>> Fixed (I use lambda expression in new webrev.)
>
>
> Ok :)
>
>>> libGetOwnedMonitorInfoTest.c
>>>
>>> Query:
>>>
>>> 64 fprintf(stderr, "MonitorContendedEnter: error in JVMTI
>>> GetThreadInfo: %d\n", err);
>>>
>>> do we not have anything to print out a more meaningful error message that
>>> the integer value of the JVMTI error code?
>>
>>
>> I added ShowErrorMessage() to show JVMTI error message.
>> It uses GetErrorName() JVMTI function.
>
>
> That looks good - thanks!
>
>>> Style query: do we do indent of 2 or 4 in hotspot C code tests ?
>>
>>
>> Fixed in C code.
>
>
> I was asking a question - I'm not sure if we use 2 or 4. Now looking at some
> other JVMTI tests they seem to use 4. I didn't do an exhaustive check to see
> if there is a predominant style.
>
> Thanks,
> David
>
>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>>> Thanks,
>>> David
>>> ------
>>>
>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> 2017-08-06 23:54 GMT+09:00 Daniel D. Daugherty
>>>> <daniel.daugherty at oracle.com>:
>>>>>
>>>>>
>>>>> On 8/6/17 1:28 AM, Yasumasa Suenaga wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Hi Dan, Serguei,
>>>>>>
>>>>>> Thanks Serguei! I can run new testcase on my environment.
>>>>>>
>>>>>> Dan, I modified your patch and uploaded as webrev:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8185164/webrev.02/
>>>>>>
>>>>>>
>>>>>> * GetOwnedMonitorInfoTest.java
>>>>>> - I use Thread#yield() to wait until MonitorContendedEnter
>>>>>> event
>>>>>> is
>>>>>> fired.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Please switch back to a small Thread.sleep(). Thread.yield()
>>>>> can be a no-op on some platforms which will make the wait
>>>>> loop very hot.
>>>>>
>>>>>
>>>>>> * libGetOwnedMonitorInfoTest.c
>>>>>> - I added "volatile" to "status" variable.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Sorry I missed that one.
>>>>>
>>>>>
>>>>>> - I moved "event_has_posted = JNI_TRUE" to each before return
>>>>>> statement.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Good catch. My original setting of event_has_posted had
>>>>> a narrow race.
>>>>>
>>>>> I'm good on the changes. I'll have test results later today.
>>>>>
>>>>> Dan
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2017/08/06 11:33, Daniel D. Daugherty wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Hi Yasumasa and Serguei,
>>>>>>>
>>>>>>> I've made some tweaks to the test and attached an updated patch:
>>>>>>>
>>>>>>> GetOwnedMonitorInfoTest.java changes:
>>>>>>>
>>>>>>> - deleted thread 't2'
>>>>>>> - made changes to make monitor contention not racy:
>>>>>>> - added 'hasEventPosted()' native function
>>>>>>> - changed Main to grab the GetOwnedMonitorInfoTest.class monitor
>>>>>>> before launching 't1'
>>>>>>> - added loop to check hasEventPosted() function while holding
>>>>>>> the GetOwnedMonitorInfoTest.class monitor
>>>>>>> - moved the delay to this new loop
>>>>>>>
>>>>>>> libGetOwnedMonitorInfoTest.c changes:
>>>>>>>
>>>>>>> - added event_has_posted flag to know when MonitorContendedEnter
>>>>>>> event has been posted
>>>>>>> - added missing error checks to JVM/TI functions
>>>>>>> - print an error message when MonitorContendedEnter or
>>>>>>> MonitorContendedEntered gets an incorrect monitor count
>>>>>>> - print error and warnings to 'stderr'
>>>>>>> - add Java_GetOwnedMonitorInfoTest_hasEventPosted function
>>>>>>>
>>>>>>> I've tested the latest version of test in a repo without the fix
>>>>>>> and it fails (as expected). Here's some sample output:
>>>>>>>
>>>>>>> ----------System.out:(7/231)----------
>>>>>>> Agent_OnLoad started
>>>>>>> Agent_OnLoad finished
>>>>>>> Main starting worker thread.
>>>>>>> Main waiting for event.
>>>>>>> Thread in sync section: Thread-1
>>>>>>> MonitorContendedEnter: Thread-1 owns 1 monitor(s)
>>>>>>> MonitorContendedEntered: Thread-1 owns 1 monitor(s)
>>>>>>> ----------System.err:(14/931)----------
>>>>>>> MonitorContendedEnter: FAIL: monitorCount should be zero.
>>>>>>> java.lang.RuntimeException: FAILED status returned from the agent
>>>>>>> at
>>>>>>> GetOwnedMonitorInfoTest.main(GetOwnedMonitorInfoTest.java:81)
>>>>>>> at
>>>>>>>
>>>>>>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
>>>>>>> Method)
>>>>>>> at
>>>>>>>
>>>>>>>
>>>>>>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>>>>>>> at
>>>>>>>
>>>>>>>
>>>>>>> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>>>>>>> at
>>>>>>> java.base/java.lang.reflect.Method.invoke(Method.java:564)
>>>>>>> at
>>>>>>>
>>>>>>>
>>>>>>> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:115)
>>>>>>> at java.base/java.lang.Thread.run(Thread.java:844)
>>>>>>>
>>>>>>> JavaTest Message: Test threw exception: java.lang.RuntimeException:
>>>>>>> FAILED status returned from the agent
>>>>>>> JavaTest Message: shutting down test
>>>>>>>
>>>>>>>
>>>>>>> Dan
>>>>>>>
>>>>>>>
>>>>>>> On 8/5/17 12:37 PM, serguei.spitsyn at oracle.com wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi Yasumasa,
>>>>>>>>
>>>>>>>> I use the following script to run the test on linux:
>>>>>>>>
>>>>>>>> % cat run.sh
>>>>>>>> #!/bin/sh
>>>>>>>> REPO=<my_repo>
>>>>>>>> IMAGES=${REPO}/build/linux-x86_64-normal-server-release/images
>>>>>>>> export JAVA_HOME=${IMAGES}/jdk
>>>>>>>> export LD_LIBRARY_PATH=${IMAGES}/test/hotspot/jtreg/native
>>>>>>>>
>>>>>>>> jtreg -J-Dtest.java.opts='-server' \
>>>>>>>> -jdk ${JAVA_HOME} \
>>>>>>>> -nativepath:${LD_LIBRARY_PATH} \
>>>>>>>> $REPO/hotspot/test/serviceability/jvmti/GetOwnedMonitorInfo
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Serguei
>>>>>>>>
>>>>>>>>
>>>>>>>> On 8/5/17 06:31, Yasumasa Suenaga wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi Serguei,
>>>>>>>>>
>>>>>>>>> I uploaded new webrev:
>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8185164/webrev.01/
>>>>>>>>>
>>>>>>>>> I tried to run serviceability/jvmti/GetOwnedMonitorInfo but it is
>>>>>>>>> failed because jtreg needs -nativepath option.
>>>>>>>>> But I didn't know what path should I set to -nativepath.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2017/08/05 20:20, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>
>>>>>>>>>> Please, merge the converted testcase.
>>>>>>>>>> It still might need more tweaks though.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Serguei
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 8/5/17 03:18, Yasumasa Suenaga wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Hi Serguei,
>>>>>>>>>>>
>>>>>>>>>>> Thank you so much!
>>>>>>>>>>> Should I merge your testcase? Or can you push this change?
>>>>>>>>>>>
>>>>>>>>>>> I agree to your change as a reviewer.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 2017/08/05 11:34, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> 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.spitsyn at oracle.com wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 8/4/17 14:26, Daniel D. Daugherty wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 8/4/17 3:17 PM, serguei.spitsyn at 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.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
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>
>
More information about the serviceability-dev
mailing list