About JDK-6772009

chen zero stzdzyhs at gmail.com
Fri Jan 6 08:52:58 UTC 2017


Hi Martin,
Thanks for the very useful links!
​I will study those first...
Regards,
chenzero


On Fri, Jan 6, 2017 at 12:27 PM, Martin Buchholz <martinrb at google.com>
wrote:

> 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