About JDK-6772009

Martin Buchholz martinrb at google.com
Fri Jan 6 04:27:30 UTC 2017


Hi chen,

The latest versions of the code lives in jsr166 CVS
http://g.oswego.edu/dl/concurrency-interest/
http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/jtreg/util/concurrent/locks/ReentrantLock/CancelledLockLoops.java?view=co
There's been work to make tests more robust that has was focused on jdk9.
There's lots of good concurrent code to study, even tests, but I don't
recommend the "Loops" jtreg tests.

On Thu, Jan 5, 2017 at 5:23 PM, chen zero <stzdzyhs at gmail.com> wrote:

> Hi Martin,
> I just curious and to get self more familiar with java.
> and that's what I try to do, simplify the test case, only waiting 1 round
> on the barrier
> In my previous email, I said I saw BrokenCarrierException, however, it's
> very rare to occur, and even I can not reproduce it now.
> anyway, I will see the latest code in jdk9....
> Thanks,
> chenzero
>
> On Fri, Jan 6, 2017 at 2:20 AM, Martin Buchholz <martinrb at google.com>
> wrote:
>
>> jtreg CancelledLockLoops is not a great test.  There have been changes to
>> it in jdk9 and it is not known to fail currently.
>>
>> But chen, why are you working on this test?  If you see it failing, try
>> simply replacing with the jdk9 version.  Otherwise, I'd like to see such
>> tests replaced with more focused understandable correctness tests rather
>> than patched.
>>
>> On Wed, Jan 4, 2017 at 9:20 PM, chen zero <stzdzyhs at gmail.com> wrote:
>>
>>> 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