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