Re: [ping] Re: RFR 8077327: ThreadStackTrace.java throws exception: BlockedThread expected to have BLOCKED but got RUNNABLE

[Jaroslav Bachorik]

On 14.4.2015 14:37, serguei.spit...@oracle.com wrote:
On 4/14/15 12:38 AM, Daniel Fuchs wrote:
Hi Jaroslav,
Shouldn't you also wait for the blockedThread to be blocked on
lockA at around line 252?
(I mean - using Utils.waitForThreadState(blockedThread, State.BLOCKED))
I guess, it is about these lines:

  255                     Utils.checkThreadState(blockedThread,
Thread.State.BLOCKED);
  256                     checkStack(blockedThread, blockedStack, bsDepth);

The implementation of the Utils.checkThreadState() from Utils.java:

     public static void checkThreadState(Thread t, Thread.State expected) {
waitForThreadState(t, expected); <=== It does the call to the
waitForThreadState

         Thread.State state = tm.getThreadInfo(t.getId()).getThreadState();
         if (state == null) {
             throw new RuntimeException(t.getName() + " expected to have " +
                 expected + " but got null.");
         }
         if (state != expected) {
             t.dumpStack();
             throw new RuntimeException(t.getName() +
                  " expected to have " + expected + " but got " + state);
         }
     }

Yes, that's true. On the other hand the original code did call 
"blockedThread.waitUntilBlocked()" explicitly - so I decided to keep the 
semantics to minimize the chance of any regressions.

-JB-


Thanks,
Serguei



best regards,

-- daniel

On 4/13/15 10:07 AM, Jaroslav Bachorik wrote:
On 9.4.2015 20:11, Jaroslav Bachorik wrote:
Please, review the following test change

Issue : https://bugs.openjdk.java.net/browse/JDK-8077327
Webrev: http://cr.openjdk.java.net/~jbachorik/8077327/webrev.00

This fix is for an intermittent failure due to timing issues. The test
is using an arbitrary waiting period to allow the tested thread to
arrive to the requested state. Usually it works fine but under eg.
heavy
load this strategy will fail. The proposed solution is to explicitly
check for the test thread arriving to the requested state instead of
waiting eg. 10ms.

I also took the liberty of removing the custom Semaphore implementation
and replacing its usage with java.util.concurrent.Phaser

Thanks,

-JB-