[ping][ping] Re: RFR 8034168: ThreadMXBean/Locks.java failed, blocked on wrong object

[Jaroslav Bachorik]

On 25.2.2014 14:45, Jaroslav Bachorik wrote:
> On 20.2.2014 18:04, Martin Buchholz wrote:
>> I think David is too pessimistic about Thread.yield being ineffective on
>> Java SE implementations (OTOH David is a Java Embedded expert).  In
>> practice an implementation that never thread switched out of a yield()
>> loop
>> would not pass the tck.  As for theory: it's true that Thread.yield
>> has no
>> progress guarantees, but then neither does Thread.sleep.  A perverse
>> implementation can always starve any thread it feels like.
>>
>> Anyways, there is nothing wrong with your sleep loop.  Except maybe you
>> want to time out eventually.
>
> The test harness should take care of timeout - at least that's what I
> can see as the preferred approach in the recent test fixes.
>
> Since David doesn't seem to have a strong opinion about Thread.sleep()
> vs. Thread.yield() here and I am rather impartial to that as well I can
> change the fix to use Thread.yield() instead if you are sure it won't
> cause any troubles.

I've changed the Thread.sleep() in the thread state checking routine to 
Thread.yield() - http://cr.openjdk.java.net/~jbachorik/8034168/webrev.01

Please, review this fix - the test fails quite often in nightlies.

Thanks,

-JB-

>
> Thanks,
>
> -JB-
>
>>
>>
>> On Thu, Feb 20, 2014 at 6:16 AM, Jaroslav Bachorik <
>> jaroslav.bachorik at oracle.com> wrote:
>>
>>> This was discussed when reviewing ThreadMXBean/
>>> SynchronizationStatistics.java
>>>
>>> Regarding a busy wait checking the thread states and issuing
>>> Thread.yield() now and then David Holmes wrote:
>>> "Not elegant and not completely reliable either. Probably adequate on a
>>> multi-core system but single-core and with some schedulers it could just
>>> be a busy spin." [1]
>>>
>>> As far as I understand, the only benefit of using Thread.yield() instead
>>> of Thread.sleep() would be 10-100ms shorter execution time, right?
>>>
>>> -JB-
>>>
>>> [1] http://mail.openjdk.java.net/pipermail/jmx-dev/2013-
>>> October/000484.html
>>>
>>>
>>> On 19.2.2014 17:56, Martin Buchholz wrote:
>>>
>>>> The jsr166 tck tests make systematic use of Thread.yield, e.g.
>>>>
>>>>
>>>>       /**
>>>>        * Spin-waits up to the specified number of milliseconds for the
>>>> given
>>>>        * thread to enter a wait state: BLOCKED, WAITING, or
>>>> TIMED_WAITING.
>>>>        */
>>>>       void waitForThreadToEnterWaitState(Thread thread, long
>>>> timeoutMillis) {
>>>>           long startTime = System.nanoTime();
>>>>           for (;;) {
>>>>               Thread.State s = thread.getState();
>>>>               if (s == Thread.State.BLOCKED ||
>>>>                   s == Thread.State.WAITING ||
>>>>                   s == Thread.State.TIMED_WAITING)
>>>>                   return;
>>>>               else if (s == Thread.State.TERMINATED)
>>>>                   fail("Unexpected thread termination");
>>>>               else if (millisElapsedSince(startTime) > timeoutMillis) {
>>>>                   threadAssertTrue(thread.isAlive());
>>>>                   return;
>>>>               }
>>>>               Thread.yield();
>>>>           }
>>>>       }
>>>>
>>>>
>>>>
>>>> On Tue, Feb 18, 2014 at 11:29 PM, Jaroslav Bachorik <
>>>> jaroslav.bachorik at oracle.com> wrote:
>>>>
>>>>   On 18.2.2014 18:06, Martin Buchholz wrote:
>>>>>
>>>>>   Not checking any details, but tests that want to wait for a
>>>>> particular
>>>>>> thread state are a good reason to use
>>>>>>
>>>>>> volatile boolean flag;
>>>>>> ...
>>>>>> while (!flag) Thread.yield();
>>>>>>
>>>>>> I prefer calling Thread.yield to sleeping in this special case, in
>>>>>> part
>>>>>> because I don't want to rely on the implementation of sleep, while
>>>>>> yield
>>>>>> is
>>>>>> semantically a no-op.  (Also sleeping 100ms is a long time for a
>>>>>> computer)
>>>>>>
>>>>>>
>>>>> There were discussions for a similar fix regarding Thread.yield(). The
>>>>> concern was that using Thread.yield() in a tight loop might very
>>>>> easily
>>>>> lead to starvation on single core machines. Therefore
>>>>> Thread.sleep(10) is
>>>>> used to be sure the flag setting thread has actually a chance to
>>>>> progress.
>>>>>
>>>>> -JB-
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> On Tue, Feb 18, 2014 at 8:22 AM, Jaroslav Bachorik <
>>>>>> jaroslav.bachorik at oracle.com> wrote:
>>>>>>
>>>>>>    Please, review the following test change.
>>>>>>
>>>>>>>
>>>>>>> Issue : https://bugs.openjdk.java.net/browse/JDK-8034168
>>>>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8034168/webrev.00
>>>>>>>
>>>>>>> The test fails because of falsely evaluating the thread being
>>>>>>> parked as
>>>>>>> actually waiting on a monitor. This is because there is no
>>>>>>> difference
>>>>>>> in
>>>>>>> java thread state for those two situations. The test is using Phaser
>>>>>>> for
>>>>>>> synchronization between the checked and checking thread to make
>>>>>>> sure an
>>>>>>> appropriate code section is entered before performing asserts.
>>>>>>> Then it
>>>>>>> checks the checked thread state and waits till it becomes WAITING.
>>>>>>> Unfortunately, when Phaser needs to wait it parks the thread and
>>>>>>> sets
>>>>>>> the
>>>>>>> thread state to WAITING. From now on the test is in a completely
>>>>>>> random
>>>>>>> state and the result will largely depend on timing - thus failing
>>>>>>> intermittently.
>>>>>>>
>>>>>>> The solution is to use an additional volatile variable to prevent
>>>>>>> falsely
>>>>>>> indicating the park() induced WAITING state.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> -JB-
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>