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

[David Holmes]

If it cures the testing failures I have no problem with it.

Thanks,
David

On 11/03/2014 6:57 PM, Jaroslav Bachorik wrote:
> 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-
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>