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 <[email protected]> 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 <[email protected]> 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 <[email protected]> >> 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 <[email protected]> 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 <[email protected]> >>>> 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 < >>>> [email protected] >>>> >> <mailto:[email protected]>> 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 >>>> >> ..... >>>> >> >>>> >> >>>> >>> >>> >> >
