About JDK-6772009
Martin Buchholz
martinrb at google.com
Thu Jan 5 18:20:54 UTC 2017
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