About JDK-6772009

chen zero stzdzyhs at gmail.com
Thu Jan 5 05:20:32 UTC 2017


Yes, the problem is related to the interrupt, timing and combine with
BrokenBarrierException, the whole testcase very complex.
so, I change the testcase as following:
    in the main:
          left 2 thread Un-interrupt.
          wait on the barrier. // if wait return, that means all threads
end.
          verify completed > 2

   in the working thread:
         try lock  and do calculation....
         if interrupted, skip calculation
         wait on the barrier.

yes, the barrier will be only wait 1 time, (in original code the barrier
will wait 2 times),
in my testing, sometime, in the 2nd time wait, BrokenBarrierException will
be thrown, however, it's not catch and make
the whole test case skipped.
anyway, two times wait on barrier are very complex....
so, only 1 time wait on barrier and in my testing, anything is as expected
:)

My code is from from jdk8u branch from git, and I compare with the code
posted in the JiRA, also same.


On Thu, Jan 5, 2017 at 12:48 PM, David Holmes <david.holmes at oracle.com>
wrote:

> Hi,
>
> You seem to be looking at an older version of the test code.
>
> As I said in my previous mail I think the problem is that an interrupted
> thread can follow this path:
>
>    if (!interrupted) {
>       if (print)
>          System.out.printf("completed after %d millis%n",
>          NANOSECONDS.toMillis(System.nanoTime() - startTime));
>       lock.lock();
>       try {
>          ++completed;
>       }
>       finally {
>          lock.unlock();
>       }
>     }
>
> because interrupted is only set when InterruptedException is caught:
>
> while (!done || Thread.currentThread().isInterrupted()) {
>     try {
>         lock.lockInterruptibly();
>     }
>     catch (InterruptedException ie) {
>         interrupted = true;
>
> but we can also exit the loop due to interruption, without recording the
> fact we were interrupted. In which case the completed count will be too
> high.
>
> Cheers,
> David
>
>
> On 5/01/2017 2:39 PM, chen zero wrote:
>
>> Hi David,
>> Thank you for reminding.
>> I post the code here.
>>
>>
>> import java.util.Arrays;
>> import java.util.Collections;
>> import java.util.Random;
>> import java.util.concurrent.BrokenBarrierException;
>> import java.util.concurrent.CyclicBarrier;
>> import java.util.concurrent.locks.ReentrantLock;
>>
>> // https://bugs.openjdk.java.net/browse/JDK-6772009
>>
>> public final class CancelledLockLoops {
>>     static final Random rng = new Random();
>>     static boolean print = false;
>>     //static final int ITERS = 1000000;
>>     static final int ITERS = 10000000;
>>
>>     static final long TIMEOUT = 100;
>>
>>     public static void main(String[] args) throws Exception {
>>         int maxThreads = 5;
>>         if (args.length > 0) {
>>             maxThreads = Integer.parseInt(args[0]);
>>         }
>>
>>         print = true;
>>
>>         for (int i = 2; i <= maxThreads; i += (i+1) >>> 1) {
>>             System.out.print("Threads: " + i);
>>
>>             try {
>>                 new ReentrantLockLoop(i).test();
>>             }
>>             catch (BrokenBarrierException bb) {
>>                 // OK, ignore
>>                 bb.printStackTrace();
>>             }
>>             Thread.sleep(TIMEOUT);
>>         }
>>     }
>>
>>     static final class ReentrantLockLoop implements Runnable {
>>         private int v = rng.nextInt();
>>         private int completed;
>>         private volatile int result = 17;
>>         private final ReentrantLock lock = new ReentrantLock();
>>         private final LoopHelpers.BarrierTimer timer = new
>> LoopHelpers.BarrierTimer();
>>         private final CyclicBarrier barrier;
>>         private final int nthreads;
>>
>>         ReentrantLockLoop(int nthreads) {
>>             this.nthreads = nthreads;
>>             barrier = new CyclicBarrier(nthreads+1, timer);
>>         }
>>
>>         void checkBarrierStatus() {
>>             if(this.barrier.isBroken()) {
>>                 throw new Error("barrier is broken !!!");
>>             }
>>         }
>>
>>         final void test() throws Exception {
>>             checkBarrierStatus();
>>
>>             timer.run();
>>
>>             Thread[] threads = new Thread[nthreads];
>>             for (int i = 0; i < threads.length; ++i) {
>>                 threads[i] = new Thread(this, "th"+i);
>>             }
>>
>>             for (int i = 0; i < threads.length; ++i) {
>>                 threads[i].start();
>>             }
>>
>>             Thread[] cancels =  (Thread[])(threads.clone());
>>
>>             Collections.shuffle(Arrays.asList(cancels), rng);
>>             Thread.sleep(TIMEOUT);
>>             for (int i = 0; i < cancels.length-2; ++i) {
>>                 cancels[i].interrupt();
>>                 // make sure all OK even when cancellations spaced out
>>                 if ((i & 3) == 0) {
>>                     Thread.sleep(1 + rng.nextInt(10));
>>                 }
>>             }
>>
>>             Thread.sleep(TIMEOUT);
>>
>>             try {
>>                 // in here, the barrier might be broken because some
>> working threads is interrupted.
>>                 // so, catching the BrokenBarrierException to let
>> testcase continue running
>>                 barrier.await();
>>             }
>>             catch(BrokenBarrierException e) {
>>                 // nop
>>             }
>>
>>             if (print) {
>>                 // since the barrier might be broken ( the barrier
>> action will not run),
>>                 // running here to ensure time is set.
>>                 timer.run();
>>                 long time = timer.getTime();
>>                 double secs = (double)(time) / 1000000000.0;
>>                 System.out.println("\t " + secs + "s run time");
>>             }
>>
>>             int c;
>>             lock.lock();
>>             //System.out.println("main to verify completed.....");
>>
>>             try {
>>                 c = completed;
>>             }
>>             finally {
>>                 lock.unlock();
>>             }
>>             // here the completed should not < 2, it can not guarantee
>> that always == 2.
>>             if (c < 2) {
>>                 throw new Error("Completed < 2 : " + c);
>>             }
>>
>>             int r = result;
>>             if (r == 0) { // avoid overoptimization
>>                 System.out.println("useless result: " + r);
>>             }
>>         }
>>
>>         public final void run() {
>>             try {
>>                 checkBarrierStatus();
>>
>>                 int sum = v;
>>                 int x = 0;
>>                 int n = ITERS;
>>                 do {
>>                     try {
>>                         lock.lockInterruptibly();
>>                     }
>>                     catch (InterruptedException ie) {
>>                         break;
>>                     }
>>                     try {
>>                         v = x = LoopHelpers.compute1(v);
>>                     }
>>                     finally {
>>                         lock.unlock();
>>                     }
>>                     sum += LoopHelpers.compute2(x);
>>                 } while (n-- > 0);
>>
>>                 /*
>>                  * in the main thread, two threads are un-interrupted,
>> others threads are interrupted.
>>                  * this line, however, can not guarantee that only
>> "un-interrupted" threads can go in,
>>                  * but also interrupted threads can go in,
>>                  * so, the "completed" will not always == 2, might be > 2
>>                  */
>>                 if (n < 0) {
>>                     lock.lock();
>>                     try {
>>                         ++completed;
>>                     }
>>                     finally {
>>                         lock.unlock();
>>                     }
>>                 }
>>
>>                 result += sum;
>>                 barrier.await();
>>             }
>>             catch(BrokenBarrierException ex) {
>>                 //ex.printStackTrace();
>>             }
>>             catch(InterruptedException ex) {
>>                 //ex.printStackTrace();
>>             }
>>             catch (Exception ex) {
>>                 ex.printStackTrace();
>>                 return;
>>             }
>>         }
>>     }
>>
>> }
>>
>>
>> On Thu, Jan 5, 2017 at 12:20 PM, David Holmes <david.holmes at oracle.com
>> <mailto:david.holmes at oracle.com>> wrote:
>>
>>     Hi chenzero,
>>
>>     Attachments get stripped from the mailing lists so you'll need to
>>     include your patch inline, or else get someone to host it for you on
>>     cr.openjdk.java.net <http://cr.openjdk.java.net>
>>
>>     Thanks,
>>     David
>>     .....
>>
>>


More information about the core-libs-dev mailing list