About JDK-6772009
David Holmes
david.holmes at oracle.com
Thu Jan 5 05:25:40 UTC 2017
Any fix would have to be applied to the current code in JDK 9.
But as this is java.util.concurrent testing Martin Buchholz will likely
jump in as he and Doug Lea maintain the external versions of these tests.
I don't think BrokenBarrierException should be an issue in current test.
David
On 5/01/2017 3:20 PM, chen zero 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
> <mailto: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
> <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>
> <mailto: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>
> <http://cr.openjdk.java.net>
>
> Thanks,
> David
> .....
>
>
More information about the core-libs-dev
mailing list