[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR 8034168: ThreadMXBean/Locks.java failed, blocked on wrong object
From:       Jaroslav Bachorik <jaroslav.bachorik () oracle ! com>
Date:       2014-02-25 13:45:13
Message-ID: 530C9E69.9070501 () oracle ! com
[Download RAW message or body]

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.

Thanks,

-JB-

>
>
> On Thu, Feb 20, 2014 at 6:16 AM, Jaroslav Bachorik <
> jaroslav.bachorik@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@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@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-
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

[prev in list] [next in list] [prev in thread] [next in thread]