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 >> >> ..... >> >> >> >> >> > >
