From 15065807664cd48ac3c05cc620d16a0181a87f28 Mon Sep 17 00:00:00 2001 From: Matthias Baesken Date: Wed, 1 Mar 2017 14:07:55 -0800 Subject: [PATCH 01/11] 8175000: jexec fails to execute simple helloworld.jar Reviewed-by: ksrini, henryjen, stuefe --- .../java.base/unix/native/launcher/jexec.c | 4 +- jdk/test/tools/launcher/Jexec.java | 80 +++++++++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 jdk/test/tools/launcher/Jexec.java diff --git a/jdk/src/java.base/unix/native/launcher/jexec.c b/jdk/src/java.base/unix/native/launcher/jexec.c index d2888234674..bd6458c67a1 100644 --- a/jdk/src/java.base/unix/native/launcher/jexec.c +++ b/jdk/src/java.base/unix/native/launcher/jexec.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 1999, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1999, 2017, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -332,7 +332,7 @@ const char * isJar(const char * path) { if (end <= count) { end -= 4; // make sure there are 4 bytes to read at start - while (start < end) { + while (start <= end) { off_t xhid = SH(buf, start); off_t xdlen = SH(buf, start + 2); diff --git a/jdk/test/tools/launcher/Jexec.java b/jdk/test/tools/launcher/Jexec.java new file mode 100644 index 00000000000..f2c9cfc37e8 --- /dev/null +++ b/jdk/test/tools/launcher/Jexec.java @@ -0,0 +1,80 @@ +/* + * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8175000 + * @summary test jexec + * @build TestHelper + * @run main Jexec + */ + +import java.io.File; +import java.io.IOException; + +public class Jexec extends TestHelper { + private final File testJar; + private final File jexecCmd; + private final String message = "Hello, do you read me ?"; + + Jexec() throws IOException { + jexecCmd = new File(JAVA_LIB, "jexec"); + if (!jexecCmd.exists() || !jexecCmd.canExecute()) { + throw new Error("jexec: does not exist or not executable"); + } + + testJar = new File("test.jar"); + StringBuilder tsrc = new StringBuilder(); + tsrc.append("public static void main(String... args) {\n"); + tsrc.append(" for (String x : args) {\n"); + tsrc.append(" System.out.println(x);\n"); + tsrc.append(" }\n"); + tsrc.append("}\n"); + createJar(testJar, tsrc.toString()); + } + + public static void main(String... args) throws Exception { + // linux is the only supported platform, give the others a pass + if (!isLinux) { + System.err.println("Warning: unsupported platform test passes vacuously"); + return; + } + // ok to run the test now + Jexec t = new Jexec(); + t.run(null); + } + + @Test + void jexec() throws Exception { + TestResult tr = doExec(jexecCmd.getAbsolutePath(), + testJar.getAbsolutePath(), message); + if (!tr.isOK()) { + System.err.println(tr); + throw new Exception("incorrect exit value"); + } + if (!tr.contains(message)) { + System.err.println(tr); + throw new Exception("expected message \'" + message + "\' not found"); + } + } +} From 908c3c315e2e0f717430c5411c1a713f8b03bc67 Mon Sep 17 00:00:00 2001 From: Doug Lea Date: Fri, 3 Mar 2017 10:45:38 -0800 Subject: [PATCH 02/11] 8141596: java/util/concurrent/ScheduledThreadPoolExecutor/GCRetention.java starts failing intermittently Reviewed-by: martin, psandoz, dholmes --- .../GCRetention.java | 83 +++++++++---------- 1 file changed, 38 insertions(+), 45 deletions(-) diff --git a/jdk/test/java/util/concurrent/ScheduledThreadPoolExecutor/GCRetention.java b/jdk/test/java/util/concurrent/ScheduledThreadPoolExecutor/GCRetention.java index 76e5eb9a5f8..f1a764b51ad 100644 --- a/jdk/test/java/util/concurrent/ScheduledThreadPoolExecutor/GCRetention.java +++ b/jdk/test/java/util/concurrent/ScheduledThreadPoolExecutor/GCRetention.java @@ -34,12 +34,12 @@ /* * @test * @summary Ensure that waiting pool threads don't retain refs to tasks. - * @library /lib/testlibrary/ */ -import static java.util.concurrent.TimeUnit.MILLISECONDS; - +import java.lang.ref.ReferenceQueue; import java.lang.ref.WeakReference; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.Delayed; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; @@ -47,10 +47,8 @@ import java.util.concurrent.RunnableScheduledFuture; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; -import jdk.testlibrary.Utils; public class GCRetention { - static final long LONG_DELAY_MS = Utils.adjustTimeout(10_000); /** * A custom thread pool with a custom RunnableScheduledFuture, for the @@ -80,53 +78,48 @@ public class GCRetention { } } - int countRefsCleared(WeakReference[] refs) { - int count = 0; - for (WeakReference ref : refs) - if (ref.get() == null) - count++; - return count; + void removeAll(ReferenceQueue q, int n) throws InterruptedException { + for (int j = n; j--> 0; ) { + if (q.poll() == null) { + for (;;) { + System.gc(); + if (q.remove(1000) != null) + break; + System.out.printf( + "%d/%d unqueued references remaining%n", j, n); + } + } + } + check(q.poll() == null); } void test(String[] args) throws Throwable { - CustomPool pool = new CustomPool(10); + final CustomPool pool = new CustomPool(10); final int size = 100; - WeakReference[] refs = new WeakReference[size]; - Future[] futures = new Future[size]; - for (int i = 0; i < size; i++) { - final Object x = new Object(); - refs[i] = new WeakReference(x); - // Create a Runnable with a strong ref to x. - Runnable r = new Runnable() { - public void run() { System.out.println(x); } - }; - // Schedule a custom task with a strong reference to r. - // Later tasks have earlier expiration, to ensure multiple - // residents of queue head. - futures[i] = pool.schedule(r, size*2-i, TimeUnit.MINUTES); - } - Thread.sleep(10); - for (int i = 0; i < size; i++) { - if (futures[i] != null) { - futures[i].cancel(false); - futures[i] = null; - } + final ReferenceQueue q = new ReferenceQueue<>(); + final List> refs = new ArrayList<>(size); + final List> futures = new ArrayList<>(size); + + // Schedule custom tasks with strong references. + class Task implements Runnable { + final Object x; + Task() { refs.add(new WeakReference<>(x = new Object(), q)); } + public void run() { System.out.println(x); } } + // Give tasks added later earlier expiration, to ensure + // multiple residents of queue head. + for (int i = size; i--> 0; ) + futures.add(pool.schedule(new Task(), i + 1, TimeUnit.MINUTES)); + futures.forEach(future -> future.cancel(false)); + futures.clear(); + pool.purge(); - int cleared = 0; - for (int i = 0; - i < 10 && (cleared = countRefsCleared(refs)) < size; - i++) { - System.gc(); - System.runFinalization(); - Thread.sleep(10); - } + removeAll(q, size); + for (WeakReference ref : refs) check(ref.get() == null); + pool.shutdown(); - pool.awaitTermination(LONG_DELAY_MS, MILLISECONDS); - if (cleared < size) - throw new Error(String.format - ("references to %d/%d tasks retained (\"leaked\")", - size - cleared, size)); + // rely on test harness to handle timeout + pool.awaitTermination(1L, TimeUnit.DAYS); } //--------------------- Infrastructure --------------------------- From c6202f9387600082fbb06b3ce112129d0b31b391 Mon Sep 17 00:00:00 2001 From: Doug Lea Date: Fri, 3 Mar 2017 10:45:38 -0800 Subject: [PATCH 03/11] 8173909: Miscellaneous changes imported from jsr166 CVS 2017-03 Reviewed-by: martin, psandoz --- .../classes/java/util/SplittableRandom.java | 3 +- .../util/concurrent/ArrayBlockingQueue.java | 1 + .../util/concurrent/ThreadLocalRandom.java | 3 +- .../locks/ReentrantReadWriteLock.java | 6 +- .../IteratorConsistency.java | 755 ----------------- .../ArrayBlockingQueue/WhiteBox.java | 759 ++++++++++++++++++ .../util/concurrent/tck/JSR166TestCase.java | 46 +- .../tck/LinkedTransferQueueTest.java | 20 +- .../java/util/concurrent/tck/PhaserTest.java | 20 +- .../util/concurrent/tck/StampedLockTest.java | 53 +- 10 files changed, 858 insertions(+), 808 deletions(-) delete mode 100644 jdk/test/java/util/concurrent/ArrayBlockingQueue/IteratorConsistency.java create mode 100644 jdk/test/java/util/concurrent/ArrayBlockingQueue/WhiteBox.java diff --git a/jdk/src/java.base/share/classes/java/util/SplittableRandom.java b/jdk/src/java.base/share/classes/java/util/SplittableRandom.java index 1a6653d56c7..f852e1dcaea 100644 --- a/jdk/src/java.base/share/classes/java/util/SplittableRandom.java +++ b/jdk/src/java.base/share/classes/java/util/SplittableRandom.java @@ -779,8 +779,7 @@ public final class SplittableRandom { * @return a stream of pseudorandom {@code double} values, * each with the given origin (inclusive) and bound (exclusive) * @throws IllegalArgumentException if {@code streamSize} is - * less than zero - * @throws IllegalArgumentException if {@code randomNumberOrigin} + * less than zero, or {@code randomNumberOrigin} * is greater than or equal to {@code randomNumberBound} */ public DoubleStream doubles(long streamSize, double randomNumberOrigin, diff --git a/jdk/src/java.base/share/classes/java/util/concurrent/ArrayBlockingQueue.java b/jdk/src/java.base/share/classes/java/util/concurrent/ArrayBlockingQueue.java index 6af7c7a98bc..4e73c087ce4 100644 --- a/jdk/src/java.base/share/classes/java/util/concurrent/ArrayBlockingQueue.java +++ b/jdk/src/java.base/share/classes/java/util/concurrent/ArrayBlockingQueue.java @@ -1226,6 +1226,7 @@ public class ArrayBlockingQueue extends AbstractQueue } else { nextIndex = NONE; nextItem = null; + if (lastRet == REMOVED) detach(); } } finally { lock.unlock(); diff --git a/jdk/src/java.base/share/classes/java/util/concurrent/ThreadLocalRandom.java b/jdk/src/java.base/share/classes/java/util/concurrent/ThreadLocalRandom.java index cfc676aa39a..22c1f9a2e29 100644 --- a/jdk/src/java.base/share/classes/java/util/concurrent/ThreadLocalRandom.java +++ b/jdk/src/java.base/share/classes/java/util/concurrent/ThreadLocalRandom.java @@ -699,8 +699,7 @@ public class ThreadLocalRandom extends Random { * @return a stream of pseudorandom {@code double} values, * each with the given origin (inclusive) and bound (exclusive) * @throws IllegalArgumentException if {@code streamSize} is - * less than zero - * @throws IllegalArgumentException if {@code randomNumberOrigin} + * less than zero, or {@code randomNumberOrigin} * is greater than or equal to {@code randomNumberBound} * @since 1.8 */ diff --git a/jdk/src/java.base/share/classes/java/util/concurrent/locks/ReentrantReadWriteLock.java b/jdk/src/java.base/share/classes/java/util/concurrent/locks/ReentrantReadWriteLock.java index d228f73d39c..ab43971e84d 100644 --- a/jdk/src/java.base/share/classes/java/util/concurrent/locks/ReentrantReadWriteLock.java +++ b/jdk/src/java.base/share/classes/java/util/concurrent/locks/ReentrantReadWriteLock.java @@ -53,16 +53,14 @@ import jdk.internal.vm.annotation.ReservedStackAccess; * *
*
Non-fair mode (default) - *
- * When constructed as non-fair (the default), the order of entry + *
When constructed as non-fair (the default), the order of entry * to the read and write lock is unspecified, subject to reentrancy * constraints. A nonfair lock that is continuously contended may * indefinitely postpone one or more reader or writer threads, but * will normally have higher throughput than a fair lock. * *
Fair mode - *
- * When constructed as fair, threads contend for entry using an + *
When constructed as fair, threads contend for entry using an * approximately arrival-order policy. When the currently held lock * is released, either the longest-waiting single writer thread will * be assigned the write lock, or if there is a group of reader threads diff --git a/jdk/test/java/util/concurrent/ArrayBlockingQueue/IteratorConsistency.java b/jdk/test/java/util/concurrent/ArrayBlockingQueue/IteratorConsistency.java deleted file mode 100644 index 0068e3804e3..00000000000 --- a/jdk/test/java/util/concurrent/ArrayBlockingQueue/IteratorConsistency.java +++ /dev/null @@ -1,755 +0,0 @@ -/* - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -/* - * This file is available under and governed by the GNU General Public - * License version 2 only, as published by the Free Software Foundation. - * However, the following notice accompanied the original version of this - * file: - * - * Written by Martin Buchholz with assistance from members of JCP - * JSR-166 Expert Group and released to the public domain, as - * explained at http://creativecommons.org/publicdomain/zero/1.0/ - */ - -import java.lang.ref.WeakReference; -import java.lang.reflect.Field; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.ArrayDeque; -import java.util.Collection; -import java.util.Collections; -import java.util.Iterator; -import java.util.List; -import java.util.NoSuchElementException; -import java.util.Queue; -import java.util.concurrent.ArrayBlockingQueue; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ThreadLocalRandom; - -/* - * @test - * @bug 7014263 - * @modules java.base/java.util.concurrent:open - * @summary White box testing of ArrayBlockingQueue iterators. - */ - -/** - * Tightly coupled to the implementation of ArrayBlockingQueue. - * Uses reflection to inspect queue and iterator state. - */ -@SuppressWarnings({"unchecked", "rawtypes"}) -public class IteratorConsistency { - final ThreadLocalRandom rnd = ThreadLocalRandom.current(); - final int CAPACITY = 20; - Field itrsField; - Field itemsField; - Field takeIndexField; - Field headField; - Field nextField; - Field prevTakeIndexField; - - void test(String[] args) throws Throwable { - itrsField = ArrayBlockingQueue.class.getDeclaredField("itrs"); - itemsField = ArrayBlockingQueue.class.getDeclaredField("items"); - takeIndexField = ArrayBlockingQueue.class.getDeclaredField("takeIndex"); - headField = Class.forName("java.util.concurrent.ArrayBlockingQueue$Itrs").getDeclaredField("head"); - nextField = Class.forName("java.util.concurrent.ArrayBlockingQueue$Itrs$Node").getDeclaredField("next"); - prevTakeIndexField = Class.forName("java.util.concurrent.ArrayBlockingQueue$Itr").getDeclaredField("prevTakeIndex"); - itrsField.setAccessible(true); - itemsField.setAccessible(true); - takeIndexField.setAccessible(true); - headField.setAccessible(true); - nextField.setAccessible(true); - prevTakeIndexField.setAccessible(true); - test(CAPACITY, true); - test(CAPACITY, false); - } - - Object itrs(ArrayBlockingQueue q) { - try { return itrsField.get(q); } - catch (Throwable ex) { throw new AssertionError(ex); } - } - - int takeIndex(ArrayBlockingQueue q) { - try { return takeIndexField.getInt(q); } - catch (Throwable ex) { throw new AssertionError(ex); } - } - - List trackedIterators(Object itrs) { - try { - List its = new ArrayList<>(); - if (itrs != null) - for (Object p = headField.get(itrs); p != null; p = nextField.get(p)) - its.add(((WeakReference)(p)).get()); - Collections.reverse(its); - return its; - } catch (Throwable ex) { throw new AssertionError(ex); } - } - - List trackedIterators(ArrayBlockingQueue q) { - return trackedIterators(itrs(q)); - } - - List attachedIterators(Object itrs) { - try { - List its = new ArrayList<>(); - if (itrs != null) - for (Object p = headField.get(itrs); p != null; p = nextField.get(p)) { - Iterator it = ((WeakReference)(p)).get(); - if (it != null && !isDetached(it)) - its.add(it); - } - Collections.reverse(its); - return its; - } catch (Throwable ex) { unexpected(ex); return null; } - } - - List attachedIterators(ArrayBlockingQueue q) { - return attachedIterators(itrs(q)); - } - - Object[] internalArray(ArrayBlockingQueue q) { - try { return (Object[]) itemsField.get(q); } - catch (Throwable ex) { throw new AssertionError(ex); } - } - - void printInternalArray(ArrayBlockingQueue q) { - System.err.println(Arrays.toString(internalArray(q))); - } - - void checkExhausted(Iterator it) { - if (rnd.nextBoolean()) { - check(!it.hasNext()); - check(isDetached(it)); - } - if (rnd.nextBoolean()) { - it.forEachRemaining(e -> { throw new AssertionError(); }); - checkDetached(it); - } - if (rnd.nextBoolean()) - try { it.next(); fail("should throw"); } - catch (NoSuchElementException success) {} - } - - boolean isDetached(Iterator it) { - try { - return prevTakeIndexField.getInt(it) < 0; - } catch (IllegalAccessException t) { unexpected(t); return false; } - } - - void checkDetached(Iterator it) { - check(isDetached(it)); - } - - void removeUsingIterator(ArrayBlockingQueue q, Object element) { - Iterator it = q.iterator(); - while (it.hasNext()) { - Object x = it.next(); - if (element.equals(x)) - it.remove(); - checkRemoveThrowsISE(it); - } - } - - void checkRemoveThrowsISE(Iterator it) { - if (rnd.nextBoolean()) - return; - try { it.remove(); fail("should throw"); } - catch (IllegalStateException success) {} - } - - void checkRemoveHasNoEffect(Iterator it, Collection c) { - if (rnd.nextBoolean()) - return; - int size = c.size(); - it.remove(); // no effect - equal(c.size(), size); - checkRemoveThrowsISE(it); - } - - void checkIterationSanity(Queue q) { - if (rnd.nextBoolean()) - return; - int size = q.size(); - Object[] a = q.toArray(); - Object[] b = new Object[size+2]; - Arrays.fill(b, Boolean.TRUE); - Object[] c = q.toArray(b); - equal(a.length, size); - check(b == c); - check(b[size] == null); - check(b[size+1] == Boolean.TRUE); - equal(q.toString(), Arrays.toString(a)); - Integer[] xx = null, yy = null; - if (size > 0) { - xx = new Integer[size - 1]; - Arrays.fill(xx, 42); - yy = ((Queue)q).toArray(xx); - for (Integer zz : xx) - equal(42, zz); - } - Iterator it = q.iterator(); - for (int i = 0; i < size; i++) { - check(it.hasNext()); - Object x = it.next(); - check(x == a[i]); - check(x == b[i]); - if (xx != null) check(x == yy[i]); - } - check(!it.hasNext()); - } - - private static void waitForFinalizersToRun() { - for (int i = 0; i < 2; i++) - tryWaitForFinalizersToRun(); - } - - private static void tryWaitForFinalizersToRun() { - System.gc(); - final CountDownLatch fin = new CountDownLatch(1); - new Object() { protected void finalize() { fin.countDown(); }}; - System.gc(); - try { fin.await(); } - catch (InterruptedException ie) { throw new Error(ie); } - } - - void test(int capacity, boolean fair) { - //---------------------------------------------------------------- - // q.clear will clear out itrs. - //---------------------------------------------------------------- - try { - ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); - List its = new ArrayList<>(); - for (int i = 0; i < capacity; i++) - check(q.add(i)); - check(itrs(q) == null); - for (int i = 0; i < capacity; i++) { - its.add(q.iterator()); - equal(trackedIterators(q), its); - q.poll(); - q.add(capacity+i); - } - q.clear(); - check(itrs(q) == null); - int j = 0; - for (Iterator it : its) { - if (rnd.nextBoolean()) - check(it.hasNext()); - equal(it.next(), j++); - checkExhausted(it); - } - } catch (Throwable t) { unexpected(t); } - - //---------------------------------------------------------------- - // q emptying will clear out itrs. - //---------------------------------------------------------------- - try { - ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); - List its = new ArrayList<>(); - for (int i = 0; i < capacity; i++) - q.add(i); - check(itrs(q) == null); - for (int i = 0; i < capacity; i++) { - its.add(q.iterator()); - equal(trackedIterators(q), its); - q.poll(); - q.add(capacity+i); - } - for (int i = 0; i < capacity; i++) - q.poll(); - check(itrs(q) == null); - int j = 0; - for (Iterator it : its) { - if (rnd.nextBoolean()) - check(it.hasNext()); - equal(it.next(), j++); - checkExhausted(it); - } - } catch (Throwable t) { unexpected(t); } - - //---------------------------------------------------------------- - // Advancing 2 cycles will remove iterators. - //---------------------------------------------------------------- - try { - ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); - List its = new ArrayList<>(); - for (int i = 0; i < capacity; i++) - q.add(i); - check(itrs(q) == null); - for (int i = capacity; i < 3 * capacity; i++) { - its.add(q.iterator()); - equal(trackedIterators(q), its); - q.poll(); - q.add(i); - } - for (int i = 3 * capacity; i < 4 * capacity; i++) { - equal(trackedIterators(q), its.subList(capacity,2*capacity)); - q.poll(); - q.add(i); - } - check(itrs(q) == null); - int j = 0; - for (Iterator it : its) { - if (rnd.nextBoolean()) - check(it.hasNext()); - equal(it.next(), j++); - checkExhausted(it); - } - } catch (Throwable t) { unexpected(t); } - - //---------------------------------------------------------------- - // Interior removal of elements used by an iterator will cause - // it to be untracked. - //---------------------------------------------------------------- - try { - ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); - q.add(0); - for (int i = 1; i < 2 * capacity; i++) { - q.add(i); - Integer[] elts = { -1, -2, -3 }; - for (Integer elt : elts) q.add(elt); - equal(q.remove(), i - 1); - Iterator it = q.iterator(); - equal(it.next(), i); - equal(it.next(), elts[0]); - Collections.shuffle(Arrays.asList(elts)); - check(q.remove(elts[0])); - check(q.remove(elts[1])); - equal(trackedIterators(q), Collections.singletonList(it)); - check(q.remove(elts[2])); - check(itrs(q) == null); - equal(it.next(), -2); - if (rnd.nextBoolean()) checkExhausted(it); - if (rnd.nextBoolean()) checkDetached(it); - } - } catch (Throwable t) { unexpected(t); } - - //---------------------------------------------------------------- - // Check iterators on an empty q - //---------------------------------------------------------------- - try { - ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); - for (int i = 0; i < 4; i++) { - Iterator it = q.iterator(); - check(itrs(q) == null); - if (rnd.nextBoolean()) checkExhausted(it); - if (rnd.nextBoolean()) checkDetached(it); - checkRemoveThrowsISE(it); - } - } catch (Throwable t) { unexpected(t); } - - //---------------------------------------------------------------- - // Check "interior" removal of iterator's last element - //---------------------------------------------------------------- - try { - ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); - List its = new ArrayList<>(); - for (int i = 0; i < capacity; i++) - q.add(i); - for (int i = 0; i < capacity; i++) { - Iterator it = q.iterator(); - its.add(it); - for (int j = 0; j < i; j++) - equal(j, it.next()); - equal(attachedIterators(q), its); - } - q.remove(capacity - 1); - equal(attachedIterators(q), its); - for (int i = 1; i < capacity - 1; i++) { - q.remove(capacity - i - 1); - Iterator it = its.get(capacity - i); - checkDetached(it); - equal(attachedIterators(q), its.subList(0, capacity - i)); - if (rnd.nextBoolean()) check(it.hasNext()); - equal(it.next(), capacity - i); - checkExhausted(it); - } - equal(attachedIterators(q), its.subList(0, 2)); - q.remove(0); - check(q.isEmpty()); - check(itrs(q) == null); - Iterator it = its.get(0); - equal(it.next(), 0); - checkRemoveHasNoEffect(it, q); - checkExhausted(it); - checkDetached(it); - checkRemoveHasNoEffect(its.get(1), q); - } catch (Throwable t) { unexpected(t); } - - //---------------------------------------------------------------- - // Check "interior" removal of alternating elements, straddling 2 cycles - //---------------------------------------------------------------- - try { - ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); - List its = new ArrayList<>(); - // Move takeIndex to middle - for (int i = 0; i < capacity/2; i++) { - check(q.add(i)); - equal(q.poll(), i); - } - check(takeIndex(q) == capacity/2); - for (int i = 0; i < capacity; i++) - q.add(i); - for (int i = 0; i < capacity; i++) { - Iterator it = q.iterator(); - its.add(it); - for (int j = 0; j < i; j++) - equal(j, it.next()); - equal(attachedIterators(q), its); - } - // Remove all even elements, in either direction using - // q.remove(), or iterator.remove() - switch (rnd.nextInt(3)) { - case 0: - for (int i = 0; i < capacity; i+=2) { - check(q.remove(i)); - equal(attachedIterators(q), its); - } - break; - case 1: - for (int i = capacity - 2; i >= 0; i-=2) { - check(q.remove(i)); - equal(attachedIterators(q), its); - } - break; - case 2: - Iterator it = q.iterator(); - while (it.hasNext()) { - int i = (Integer) it.next(); - if ((i & 1) == 0) - it.remove(); - } - equal(attachedIterators(q), its); - break; - default: throw new AssertionError(); - } - - for (int i = 0; i < capacity; i++) { - Iterator it = its.get(i); - boolean even = ((i & 1) == 0); - if (even) { - if (rnd.nextBoolean()) check(it.hasNext()); - equal(i, it.next()); - for (int j = i+1; j < capacity; j += 2) - equal(j, it.next()); - check(!isDetached(it)); - check(!it.hasNext()); - check(isDetached(it)); - } else { /* odd */ - if (rnd.nextBoolean()) check(it.hasNext()); - checkRemoveHasNoEffect(it, q); - equal(i, it.next()); - for (int j = i+2; j < capacity; j += 2) - equal(j, it.next()); - check(!isDetached(it)); - check(!it.hasNext()); - check(isDetached(it)); - } - } - equal(trackedIterators(q), Collections.emptyList()); - check(itrs(q) == null); - } catch (Throwable t) { unexpected(t); } - - //---------------------------------------------------------------- - // Check garbage collection of discarded iterators - //---------------------------------------------------------------- - try { - ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); - List its = new ArrayList<>(); - for (int i = 0; i < capacity; i++) - q.add(i); - for (int i = 0; i < capacity; i++) { - its.add(q.iterator()); - equal(attachedIterators(q), its); - } - its = null; - waitForFinalizersToRun(); - List trackedIterators = trackedIterators(q); - equal(trackedIterators.size(), capacity); - for (Iterator x : trackedIterators) - check(x == null); - Iterator it = q.iterator(); - equal(trackedIterators(q), Collections.singletonList(it)); - } catch (Throwable t) { unexpected(t); } - - //---------------------------------------------------------------- - // Check garbage collection of discarded iterators, - // with a randomly retained subset. - //---------------------------------------------------------------- - try { - ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); - List its = new ArrayList<>(); - List retained = new ArrayList<>(); - final int size = 1 + rnd.nextInt(capacity); - for (int i = 0; i < size; i++) - q.add(i); - for (int i = 0; i < size; i++) { - Iterator it = q.iterator(); - its.add(it); - equal(attachedIterators(q), its); - } - // Leave sufficient gaps in retained - for (int i = 0; i < size; i+= 2+rnd.nextInt(3)) - retained.add(its.get(i)); - its = null; - waitForFinalizersToRun(); - List trackedIterators = trackedIterators(q); - equal(trackedIterators.size(), size); - for (Iterator it : trackedIterators) - check((it == null) ^ retained.contains(it)); - Iterator it = q.iterator(); // trigger another sweep - retained.add(it); - equal(trackedIterators(q), retained); - } catch (Throwable t) { unexpected(t); } - - //---------------------------------------------------------------- - // Check incremental sweeping of discarded iterators. - // Excessively white box?! - //---------------------------------------------------------------- - try { - final int SHORT_SWEEP_PROBES = 4; - final int LONG_SWEEP_PROBES = 16; - final int PROBE_HOP = LONG_SWEEP_PROBES + 6 * SHORT_SWEEP_PROBES; - final int PROBE_HOP_COUNT = 10; - // Expect around 8 sweeps per PROBE_HOP - final int SWEEPS_PER_PROBE_HOP = 8; - ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); - List its = new ArrayList<>(); - for (int i = 0; i < capacity; i++) - q.add(i); - for (int i = 0; i < PROBE_HOP_COUNT * PROBE_HOP; i++) { - its.add(q.iterator()); - equal(attachedIterators(q), its); - } - // make some garbage, separated by PROBE_HOP - for (int i = 0; i < its.size(); i += PROBE_HOP) - its.set(i, null); - waitForFinalizersToRun(); - int retries; - for (retries = 0; - trackedIterators(q).contains(null) && retries < 1000; - retries++) - // one round of sweeping - its.add(q.iterator()); - check(retries >= PROBE_HOP_COUNT * (SWEEPS_PER_PROBE_HOP - 2)); - check(retries <= PROBE_HOP_COUNT * (SWEEPS_PER_PROBE_HOP + 2)); - Iterator itsit = its.iterator(); - while (itsit.hasNext()) - if (itsit.next() == null) - itsit.remove(); - equal(trackedIterators(q), its); - } catch (Throwable t) { unexpected(t); } - - //---------------------------------------------------------------- - // Check safety of iterator.remove while in detached mode. - //---------------------------------------------------------------- - try { - ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); - List its = new ArrayList<>(); - for (int i = 0; i < capacity/2; i++) { - q.add(i); - q.remove(); - } - check(takeIndex(q) == capacity/2); - for (int i = 0; i < capacity; i++) - q.add(i); - for (int i = 0; i < capacity; i++) { - Iterator it = q.iterator(); - its.add(it); - for (int j = 0; j < i; j++) - equal(j, it.next()); - equal(attachedIterators(q), its); - } - for (int i = capacity - 1; i >= 0; i--) { - Iterator it = its.get(i); - equal(i, it.next()); // last element - check(!isDetached(it)); - check(!it.hasNext()); // first hasNext failure - check(isDetached(it)); - int size = q.size(); - check(q.contains(i)); - switch (rnd.nextInt(3)) { - case 0: - it.remove(); - check(!q.contains(i)); - equal(q.size(), size - 1); - break; - case 1: - // replace i with impostor - if (q.remainingCapacity() == 0) { - check(q.remove(i)); - check(q.add(-1)); - } else { - check(q.add(-1)); - check(q.remove(i)); - } - it.remove(); // should have no effect - equal(size, q.size()); - check(q.contains(-1)); - check(q.remove(-1)); - break; - case 2: - // replace i with true impostor - if (i != 0) { - check(q.remove(i)); - check(q.add(i)); - } - it.remove(); - check(!q.contains(i)); - equal(q.size(), size - 1); - break; - default: throw new AssertionError(); - } - checkRemoveThrowsISE(it); - check(isDetached(it)); - check(!trackedIterators(q).contains(it)); - } - check(q.isEmpty()); - check(itrs(q) == null); - for (Iterator it : its) - checkExhausted(it); - } catch (Throwable t) { unexpected(t); } - - //---------------------------------------------------------------- - // Check dequeues bypassing iterators' current positions. - //---------------------------------------------------------------- - try { - ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); - Queue its0 = new ArrayDeque<>(); - Queue itsMid = new ArrayDeque<>(); - List its = new ArrayList<>(); - for (int i = 0; i < capacity; i++) - q.add(i); - for (int i = 0; i < 2 * capacity + 1; i++) { - Iterator it = q.iterator(); - its.add(it); - its0.add(it); - } - for (int i = 0; i < 2 * capacity + 1; i++) { - Iterator it = q.iterator(); - for (int j = 0; j < capacity/2; j++) - equal(j, it.next()); - its.add(it); - itsMid.add(it); - } - for (int i = capacity; i < 3 * capacity; i++) { - Iterator it; - - it = its0.remove(); - checkRemoveThrowsISE(it); - if (rnd.nextBoolean()) check(it.hasNext()); - equal(0, it.next()); - int victim = i - capacity; - for (int j = victim + (victim == 0 ? 1 : 0); j < i; j++) { - if (rnd.nextBoolean()) check(it.hasNext()); - equal(j, it.next()); - } - checkExhausted(it); - - it = itsMid.remove(); - if (victim >= capacity/2) - checkRemoveHasNoEffect(it, q); - equal(capacity/2, it.next()); - if (victim > capacity/2) - checkRemoveHasNoEffect(it, q); - for (int j = Math.max(victim, capacity/2 + 1); j < i; j++) { - if (rnd.nextBoolean()) check(it.hasNext()); - equal(j, it.next()); - } - checkExhausted(it); - - if (rnd.nextBoolean()) { - equal(victim, q.remove()); - } else { - ArrayList list = new ArrayList(1); - q.drainTo(list, 1); - equal(list.size(), 1); - equal(victim, list.get(0)); - } - check(q.add(i)); - } - // takeIndex has wrapped twice. - Iterator it0 = its0.remove(); - Iterator itMid = itsMid.remove(); - check(isDetached(it0)); - check(isDetached(itMid)); - if (rnd.nextBoolean()) check(it0.hasNext()); - if (rnd.nextBoolean()) check(itMid.hasNext()); - checkRemoveThrowsISE(it0); - checkRemoveHasNoEffect(itMid, q); - if (rnd.nextBoolean()) equal(0, it0.next()); - if (rnd.nextBoolean()) equal(capacity/2, itMid.next()); - check(isDetached(it0)); - check(isDetached(itMid)); - equal(capacity, q.size()); - equal(0, q.remainingCapacity()); - } catch (Throwable t) { unexpected(t); } - - //---------------------------------------------------------------- - // Check collective sanity of iteration, toArray() and toString() - //---------------------------------------------------------------- - try { - ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); - for (int i = 0; i < capacity; i++) { - checkIterationSanity(q); - equal(capacity, q.size() + q.remainingCapacity()); - q.add(i); - } - for (int i = 0; i < (capacity + (capacity >> 1)); i++) { - checkIterationSanity(q); - equal(capacity, q.size() + q.remainingCapacity()); - equal(i, q.peek()); - equal(i, q.poll()); - checkIterationSanity(q); - equal(capacity, q.size() + q.remainingCapacity()); - q.add(capacity + i); - } - for (int i = 0; i < capacity; i++) { - checkIterationSanity(q); - equal(capacity, q.size() + q.remainingCapacity()); - int expected = i + capacity + (capacity >> 1); - equal(expected, q.peek()); - equal(expected, q.poll()); - } - checkIterationSanity(q); - } catch (Throwable t) { unexpected(t); } - - } - - //--------------------- Infrastructure --------------------------- - volatile int passed = 0, failed = 0; - void pass() {passed++;} - void fail() {failed++; Thread.dumpStack();} - void fail(String msg) {System.err.println(msg); fail();} - void unexpected(Throwable t) {failed++; t.printStackTrace();} - void check(boolean cond) {if (cond) pass(); else fail();} - void equal(Object x, Object y) { - if (x == null ? y == null : x.equals(y)) pass(); - else fail(x + " not equal to " + y);} - public static void main(String[] args) throws Throwable { - new IteratorConsistency().instanceMain(args);} - public void instanceMain(String[] args) throws Throwable { - try {test(args);} catch (Throwable t) {unexpected(t);} - System.err.printf("%nPassed = %d, failed = %d%n%n", passed, failed); - if (failed > 0) throw new AssertionError("Some tests failed");} -} diff --git a/jdk/test/java/util/concurrent/ArrayBlockingQueue/WhiteBox.java b/jdk/test/java/util/concurrent/ArrayBlockingQueue/WhiteBox.java new file mode 100644 index 00000000000..890748faa5b --- /dev/null +++ b/jdk/test/java/util/concurrent/ArrayBlockingQueue/WhiteBox.java @@ -0,0 +1,759 @@ +/* + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * This file is available under and governed by the GNU General Public + * License version 2 only, as published by the Free Software Foundation. + * However, the following notice accompanied the original version of this + * file: + * + * Written by Martin Buchholz with assistance from members of JCP + * JSR-166 Expert Group and released to the public domain, as + * explained at http://creativecommons.org/publicdomain/zero/1.0/ + */ + +/* + * @test + * @modules java.base/java.util.concurrent:open + * @run testng WhiteBox + * @summary White box tests of implementation details + */ + +import static org.testng.Assert.*; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +import java.lang.ref.WeakReference; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.VarHandle; +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.NoSuchElementException; +import java.util.Queue; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.TimeUnit; +import java.util.function.BooleanSupplier; + +@Test +public class WhiteBox { + final ThreadLocalRandom rnd = ThreadLocalRandom.current(); + final MethodHandles.Lookup lookup = MethodHandles.lookup(); + final VarHandle ITRS, ITEMS, TAKEINDEX, PUTINDEX, COUNT, HEAD, NEXT, PREVTAKEINDEX; + + WhiteBox() throws ReflectiveOperationException { + Class qClass = ArrayBlockingQueue.class; + Class itrClass = Class.forName(qClass.getName() + "$Itr"); + Class itrsClass = Class.forName(qClass.getName() + "$Itrs"); + Class nodeClass = Class.forName(itrsClass.getName() + "$Node"); + ITRS = findVarHandle(qClass, "itrs", itrsClass); + ITEMS = findVarHandle(qClass, "items", Object[].class); + TAKEINDEX = findVarHandle(qClass, "takeIndex", int.class); + PUTINDEX = findVarHandle(qClass, "putIndex", int.class); + COUNT = findVarHandle(qClass, "count", int.class); + HEAD = findVarHandle(itrsClass, "head", nodeClass); + NEXT = findVarHandle(nodeClass, "next", nodeClass); + PREVTAKEINDEX = findVarHandle(itrClass, "prevTakeIndex", int.class); + } + + VarHandle findVarHandle(Class recv, String name, Class type) + throws ReflectiveOperationException { + return MethodHandles.privateLookupIn(recv, lookup) + .findVarHandle(recv, name, type); + } + + Object itrs(ArrayBlockingQueue q) { return ITRS.get(q); } + Object[] items(ArrayBlockingQueue q) { return (Object[]) ITEMS.get(q); } + int takeIndex(ArrayBlockingQueue q) { return (int) TAKEINDEX.get(q); } + int putIndex(ArrayBlockingQueue q) { return (int) PUTINDEX.get(q); } + int count(ArrayBlockingQueue q) { return (int) COUNT.get(q); } + Object head(Object itrs) { return HEAD.get(itrs); } + Object next(Object node) { return NEXT.get(node); } + int prevTakeIndex(Iterator itr) { return (int) PREVTAKEINDEX.get(itr); } + + boolean isDetached(Iterator it) { + return prevTakeIndex(it) < 0; + } + + void assertIteratorExhausted(Iterator it) { + if (rnd.nextBoolean()) { + assertTrue(!it.hasNext()); + assertTrue(isDetached(it)); + } + if (rnd.nextBoolean()) { + it.forEachRemaining(e -> { throw new AssertionError(); }); + assertTrue(isDetached(it)); + } + if (rnd.nextBoolean()) + try { it.next(); fail("should throw"); } + catch (NoSuchElementException success) {} + } + + List trackedIterators(ArrayBlockingQueue q) { + List its = new ArrayList<>(); + Object itrs = itrs(q); + if (itrs != null) { + for (Object p = head(itrs); p != null; p = next(p)) + its.add(((WeakReference)(p)).get()); + Collections.reverse(its); + } + return its; + } + + List attachedIterators(ArrayBlockingQueue q) { + List its = new ArrayList<>(); + Object itrs = itrs(q); + if (itrs != null) { + for (Object p = head(itrs); p != null; p = next(p)) { + Iterator it = ((WeakReference)(p)).get(); + if (it != null && !isDetached(it)) + its.add(it); + } + Collections.reverse(its); + } + return its; + } + + void assertRemoveThrowsISE(Iterator it) { + if (rnd.nextBoolean()) + try { it.remove(); fail("should throw"); } + catch (IllegalStateException success) {} + } + + void assertRemoveHasNoEffect(Iterator it, Collection c) { + if (rnd.nextBoolean()) { + int size = c.size(); + it.remove(); // no effect + assertEquals(c.size(), size); + assertRemoveThrowsISE(it); + } + } + + void checkIterationSanity(Queue q) { + if (rnd.nextBoolean()) + return; + int size = q.size(); + Object[] a = q.toArray(); + Object[] b = new Object[size+2]; + Arrays.fill(b, Boolean.TRUE); + Object[] c = q.toArray(b); + assertEquals(a.length, size); + assertSame(b, c); + assertNull(b[size]); + assertSame(b[size+1], Boolean.TRUE); + assertEquals(q.toString(), Arrays.toString(a)); + Integer[] xx = null, yy = null; + if (size > 0) { + xx = new Integer[size - 1]; + Arrays.fill(xx, 42); + yy = ((Queue)q).toArray(xx); + for (Integer zz : xx) + assertEquals(42, (int) zz); + } + Iterator it = q.iterator(); + for (int i = 0; i < size; i++) { + if (rnd.nextBoolean()) assertTrue(it.hasNext()); + Object x = it.next(); + assertSame(x, a[i]); + assertSame(x, b[i]); + if (xx != null) assertSame(x, yy[i]); + } + if (rnd.nextBoolean()) assertTrue(!it.hasNext()); + } + + /** + * Instead of having putIndex (and takeIndex) at the initial + * default of 0, move them to a random location. + */ + void randomizePutIndex(ArrayBlockingQueue q) { + assertTrue(q.isEmpty()); + int capacity = q.remainingCapacity(); + int n = rnd.nextInt(capacity + 1); + int putIndex = putIndex(q); + for (int i = n; i-->0; ) q.add(Boolean.TRUE); + for (int i = n; i-->0; ) q.remove(); + assertEquals(putIndex(q), (putIndex + n) % items(q).length); + } + + /** No guarantees, but effective in practice. */ + static void forceFullGc() { + CountDownLatch finalizeDone = new CountDownLatch(1); + WeakReference ref = new WeakReference(new Object() { + protected void finalize() { finalizeDone.countDown(); }}); + try { + for (int i = 0; i < 10; i++) { + System.gc(); + if (finalizeDone.await(1L, TimeUnit.SECONDS) && ref.get() == null) { + System.runFinalization(); // try to pick up stragglers + return; + } + } + } catch (InterruptedException unexpected) { + throw new AssertionError("unexpected InterruptedException"); + } + throw new AssertionError("failed to do a \"full\" gc"); + } + + static void gcAwait(BooleanSupplier s) { + for (int i = 0; i < 10; i++) { + if (s.getAsBoolean()) + return; + forceFullGc(); + } + throw new AssertionError("failed to satisfy condition"); + } + + public void clear_willClearItrs() { + boolean fair = rnd.nextBoolean(); + int capacity = rnd.nextInt(2, 10); + ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); + randomizePutIndex(q); + List its = new ArrayList<>(); + for (int i = 0; i < capacity; i++) + assertTrue(q.add(i)); + assertNull(itrs(q)); + for (int i = 0; i < capacity; i++) { + its.add(q.iterator()); + assertEquals(trackedIterators(q), its); + q.poll(); + q.add(capacity + i); + } + q.clear(); + assertNull(itrs(q)); + int j = 0; + for (Iterator it : its) { + assertTrue(isDetached(it)); + if (rnd.nextBoolean()) assertTrue(it.hasNext()); + if (rnd.nextBoolean()) { + assertEquals(it.next(), j); + assertIteratorExhausted(it); + } + j++; + } + } + + public void queueEmptyingWillClearItrs() { + boolean fair = rnd.nextBoolean(); + int capacity = rnd.nextInt(2, 10); + ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); + randomizePutIndex(q); + List its = new ArrayList<>(); + for (int i = 0; i < capacity; i++) + q.add(i); + assertNull(itrs(q)); + for (int i = 0; i < capacity; i++) { + its.add(q.iterator()); + assertEquals(trackedIterators(q), its); + q.poll(); + q.add(capacity+i); + } + for (int i = 0; i < capacity; i++) + q.poll(); + assertNull(itrs(q)); + int j = 0; + for (Iterator it : its) { + assertTrue(isDetached(it)); + if (rnd.nextBoolean()) assertTrue(it.hasNext()); + if (rnd.nextBoolean()) { + assertEquals(it.next(), j); + assertIteratorExhausted(it); + } + j++; + } + } + + public void advancing2CyclesWillRemoveIterators() { + boolean fair = rnd.nextBoolean(); + int capacity = rnd.nextInt(2, 10); + ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); + List its = new ArrayList<>(); + for (int i = 0; i < capacity; i++) + q.add(i); + assertNull(itrs(q)); + for (int i = capacity; i < 3 * capacity; i++) { + its.add(q.iterator()); + assertEquals(trackedIterators(q), its); + q.poll(); + q.add(i); + } + for (int i = 3 * capacity; i < 4 * capacity; i++) { + assertEquals(trackedIterators(q), its.subList(capacity,2*capacity)); + q.poll(); + q.add(i); + } + assertNull(itrs(q)); + int j = 0; + for (Iterator it : its) { + assertTrue(isDetached(it)); + if (rnd.nextBoolean()) assertTrue(it.hasNext()); + if (rnd.nextBoolean()) { + assertEquals(it.next(), j); + assertIteratorExhausted(it); + } + j++; + } + } + + /** + * Interior removal of elements used by an iterator will cause it + * to be untracked. + */ + public void interiorRemovalOfElementsUsedByIterator() { + boolean fair = rnd.nextBoolean(); + int capacity = rnd.nextInt(10, 20); + ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); + randomizePutIndex(q); + q.add(0); + for (int i = 1; i < 2 * capacity; i++) { + q.add(i); + Integer[] elts = { -1, -2, -3 }; + for (Integer elt : elts) q.add(elt); + assertEquals(q.remove(), i - 1); + Iterator it = q.iterator(); + assertEquals(it.next(), i); + assertEquals(it.next(), elts[0]); + Collections.shuffle(Arrays.asList(elts)); + assertTrue(q.remove(elts[0])); + assertTrue(q.remove(elts[1])); + assertEquals(trackedIterators(q), Collections.singletonList(it)); + assertTrue(q.remove(elts[2])); + assertNull(itrs(q)); + assertEquals(it.next(), -2); + assertIteratorExhausted(it); + assertTrue(isDetached(it)); + } + } + + public void iteratorsOnEmptyQueue() { + boolean fair = rnd.nextBoolean(); + int capacity = rnd.nextInt(1, 10); + ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); + randomizePutIndex(q); + for (int i = 0; i < 4; i++) { + Iterator it = q.iterator(); + assertNull(itrs(q)); + assertIteratorExhausted(it); + assertTrue(isDetached(it)); + assertRemoveThrowsISE(it); + } + } + + public void interiorRemovalOfIteratorsLastElement() { + boolean fair = rnd.nextBoolean(); + int capacity = rnd.nextInt(3, 10); + ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); + randomizePutIndex(q); + List its = new ArrayList<>(); + for (int i = 0; i < capacity; i++) + q.add(i); + for (int i = 0; i < capacity; i++) { + Iterator it = q.iterator(); + its.add(it); + for (int j = 0; j < i; j++) + assertEquals(j, it.next()); + assertEquals(attachedIterators(q), its); + } + q.remove(capacity - 1); + assertEquals(attachedIterators(q), its); + for (int i = 1; i < capacity - 1; i++) { + q.remove(capacity - i - 1); + Iterator it = its.get(capacity - i); + assertTrue(isDetached(it)); + assertEquals(attachedIterators(q), its.subList(0, capacity - i)); + if (rnd.nextBoolean()) assertTrue(it.hasNext()); + assertEquals(it.next(), capacity - i); + assertIteratorExhausted(it); + } + assertEquals(attachedIterators(q), its.subList(0, 2)); + q.remove(0); + assertTrue(q.isEmpty()); + assertNull(itrs(q)); + Iterator it = its.get(0); + assertEquals(it.next(), 0); + assertRemoveHasNoEffect(it, q); + assertIteratorExhausted(it); + assertTrue(isDetached(it)); + assertRemoveHasNoEffect(its.get(1), q); + } + + /** + * Checks "interior" removal of alternating elements, straddling 2 cycles + */ + public void interiorRemovalOfAlternatingElements() { + boolean fair = rnd.nextBoolean(); + int capacity = 2 * rnd.nextInt(2, 10); + ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); + List its = new ArrayList<>(); + // Move takeIndex to middle + for (int i = 0; i < capacity/2; i++) { + assertTrue(q.add(i)); + assertEquals(q.poll(), i); + } + assertEquals(takeIndex(q), capacity/2); + for (int i = 0; i < capacity; i++) + q.add(i); + for (int i = 0; i < capacity; i++) { + Iterator it = q.iterator(); + its.add(it); + for (int j = 0; j < i; j++) + assertEquals(j, it.next()); + assertEquals(attachedIterators(q), its); + } + + // Remove all even elements, in either direction, using + // q.remove(), or iterator.remove() + switch (rnd.nextInt(3)) { + case 0: + for (int i = 0; i < capacity; i+=2) assertTrue(q.remove(i)); + break; + case 1: + for (int i = capacity - 2; i >= 0; i-=2) assertTrue(q.remove(i)); + break; + case 2: + Iterator it = q.iterator(); + while (it.hasNext()) { + int i = (Integer) it.next(); + if ((i & 1) == 0) + it.remove(); + } + break; + default: throw new AssertionError(); + } + assertEquals(attachedIterators(q), its); + + for (int i = 0; i < capacity; i++) { + Iterator it = its.get(i); + boolean even = ((i & 1) == 0); + if (even) { + if (rnd.nextBoolean()) assertTrue(it.hasNext()); + assertEquals(i, it.next()); + for (int j = i+1; j < capacity; j += 2) + assertEquals(j, it.next()); + assertTrue(!isDetached(it)); + assertTrue(!it.hasNext()); + assertTrue(isDetached(it)); + } else { /* odd */ + if (rnd.nextBoolean()) assertTrue(it.hasNext()); + assertRemoveHasNoEffect(it, q); + assertEquals(i, it.next()); + for (int j = i+2; j < capacity; j += 2) + assertEquals(j, it.next()); + assertTrue(!isDetached(it)); + assertTrue(!it.hasNext()); + assertTrue(isDetached(it)); + } + } + assertEquals(trackedIterators(q), Collections.emptyList()); + assertNull(itrs(q)); + } + + public void garbageCollectionOfUnreachableIterators() { + boolean fair = rnd.nextBoolean(); + int capacity = rnd.nextInt(1, 10); + ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); + randomizePutIndex(q); + List its = new ArrayList<>(); + for (int i = 0; i < capacity; i++) q.add(i); + for (int i = 0; i < capacity; i++) its.add(q.iterator()); + assertEquals(attachedIterators(q), its); + its = null; + gcAwait(() -> { + List trackedIterators = trackedIterators(q); + assertEquals(trackedIterators.size(), capacity); + for (Iterator x : trackedIterators) + if (x != null) return false; + return true; + }); + Iterator it = q.iterator(); // + assertEquals(trackedIterators(q), Collections.singletonList(it)); + } + + public void garbageCollectionOfUnreachableIteratorsWithRandomlyRetainedSubset() { + boolean fair = rnd.nextBoolean(); + int capacity = rnd.nextInt(10, 20); + ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); + randomizePutIndex(q); + List its = new ArrayList<>(); + List retained = new ArrayList<>(); + final int size = 1 + rnd.nextInt(capacity); + for (int i = 0; i < size; i++) q.add(i); + for (int i = 0; i < size; i++) its.add(q.iterator()); + assertEquals(attachedIterators(q), its); + // Leave sufficient gaps in retained + for (int i = 0; i < size; i+= 2+rnd.nextInt(3)) + retained.add(its.get(i)); + its = null; + gcAwait(() -> { + List trackedIterators = trackedIterators(q); + assertEquals(trackedIterators.size(), size); + for (Iterator it : trackedIterators) + if ((it != null) ^ retained.contains(it)) return false; + return true; + }); + Iterator it = q.iterator(); // trigger another sweep + retained.add(it); + assertEquals(trackedIterators(q), retained); + } + + /** + * Checks incremental sweeping of unreachable iterators. + * Excessively white box?! + */ + public void incrementalSweepingOfUnreachableIterators() { + boolean fair = rnd.nextBoolean(); + int capacity = rnd.nextInt(10, 20); + ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); + randomizePutIndex(q); + final int SHORT_SWEEP_PROBES = 4; + final int LONG_SWEEP_PROBES = 16; + final int PROBE_HOP = LONG_SWEEP_PROBES + 6 * SHORT_SWEEP_PROBES; + final int PROBE_HOP_COUNT = 10; + // Expect around 8 sweeps per PROBE_HOP + final int SWEEPS_PER_PROBE_HOP = 8; + List its = new ArrayList<>(); + for (int i = 0; i < capacity; i++) + q.add(i); + for (int i = 0; i < PROBE_HOP_COUNT * PROBE_HOP; i++) + its.add(q.iterator()); + assertEquals(attachedIterators(q), its); + // make some garbage, separated by PROBE_HOP + for (int i = 0; i < its.size(); i += PROBE_HOP) + its.set(i, null); + its.removeIf(it -> it == null); + forceFullGc(); + int retries; + for (retries = 0; + trackedIterators(q).contains(null) && retries < 1000; + retries++) + // one round of sweeping + its.add(q.iterator()); + assertTrue(retries >= PROBE_HOP_COUNT * (SWEEPS_PER_PROBE_HOP - 2)); + assertTrue(retries <= PROBE_HOP_COUNT * (SWEEPS_PER_PROBE_HOP + 2)); + assertEquals(trackedIterators(q), its); + } + + public void Iterator_remove_safetyWhileInDetachedMode() { + boolean fair = rnd.nextBoolean(); + int capacity = rnd.nextInt(10, 20); + ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); + List its = new ArrayList<>(); + for (int i = 0; i < capacity/2; i++) { + q.add(i); + q.remove(); + } + assertEquals(takeIndex(q), capacity/2); + for (int i = 0; i < capacity; i++) + q.add(i); + for (int i = 0; i < capacity; i++) { + Iterator it = q.iterator(); + its.add(it); + for (int j = 0; j < i; j++) + assertEquals(j, it.next()); + } + assertEquals(attachedIterators(q), its); + for (int i = capacity - 1; i >= 0; i--) { + Iterator it = its.get(i); + assertEquals(i, it.next()); // last element + assertTrue(!isDetached(it)); + assertTrue(!it.hasNext()); // first hasNext failure + assertTrue(isDetached(it)); + int size = q.size(); + assertTrue(q.contains(i)); + switch (rnd.nextInt(3)) { + case 0: + it.remove(); + assertTrue(!q.contains(i)); + assertEquals(q.size(), size - 1); + break; + case 1: + // replace i with impostor + if (q.remainingCapacity() == 0) { + assertTrue(q.remove(i)); + assertTrue(q.add(-1)); + } else { + assertTrue(q.add(-1)); + assertTrue(q.remove(i)); + } + it.remove(); // should have no effect + assertEquals(size, q.size()); + assertTrue(q.contains(-1)); + assertTrue(q.remove(-1)); + break; + case 2: + // replace i with true impostor + if (i != 0) { + assertTrue(q.remove(i)); + assertTrue(q.add(i)); + } + it.remove(); + assertTrue(!q.contains(i)); + assertEquals(q.size(), size - 1); + break; + default: throw new AssertionError(); + } + assertRemoveThrowsISE(it); + assertTrue(isDetached(it)); + assertTrue(!trackedIterators(q).contains(it)); + } + assertTrue(q.isEmpty()); + assertNull(itrs(q)); + for (Iterator it : its) + assertIteratorExhausted(it); + } + + /** + * Checks dequeues bypassing iterators' current positions. + */ + public void dequeuesBypassingIteratorCurrentPositions() { + boolean fair = rnd.nextBoolean(); + int capacity = rnd.nextInt(10, 20); + ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); + Queue its0 = new ArrayDeque<>(); + Queue itsMid = new ArrayDeque<>(); + List its = new ArrayList<>(); + for (int i = 0; i < capacity; i++) + q.add(i); + for (int i = 0; i < 2 * capacity + 1; i++) { + Iterator it = q.iterator(); + its.add(it); + its0.add(it); + } + for (int i = 0; i < 2 * capacity + 1; i++) { + Iterator it = q.iterator(); + for (int j = 0; j < capacity/2; j++) + assertEquals(j, it.next()); + its.add(it); + itsMid.add(it); + } + for (int i = capacity; i < 3 * capacity; i++) { + Iterator it; + + it = its0.remove(); + assertRemoveThrowsISE(it); + if (rnd.nextBoolean()) assertTrue(it.hasNext()); + assertEquals(0, it.next()); + int victim = i - capacity; + for (int j = victim + (victim == 0 ? 1 : 0); j < i; j++) { + if (rnd.nextBoolean()) assertTrue(it.hasNext()); + assertEquals(j, it.next()); + } + assertIteratorExhausted(it); + + it = itsMid.remove(); + if (victim >= capacity/2) + assertRemoveHasNoEffect(it, q); + assertEquals(capacity/2, it.next()); + if (victim > capacity/2) + assertRemoveHasNoEffect(it, q); + for (int j = Math.max(victim, capacity/2 + 1); j < i; j++) { + if (rnd.nextBoolean()) assertTrue(it.hasNext()); + assertEquals(j, it.next()); + } + assertIteratorExhausted(it); + + if (rnd.nextBoolean()) { + assertEquals(victim, q.remove()); + } else { + ArrayList list = new ArrayList(1); + q.drainTo(list, 1); + assertEquals(list.size(), 1); + assertEquals(victim, list.get(0)); + } + assertTrue(q.add(i)); + } + // takeIndex has wrapped twice. + Iterator it0 = its0.remove(); + Iterator itMid = itsMid.remove(); + assertTrue(isDetached(it0)); + assertTrue(isDetached(itMid)); + if (rnd.nextBoolean()) assertTrue(it0.hasNext()); + if (rnd.nextBoolean()) assertTrue(itMid.hasNext()); + assertRemoveThrowsISE(it0); + assertRemoveHasNoEffect(itMid, q); + if (rnd.nextBoolean()) assertEquals(0, it0.next()); + if (rnd.nextBoolean()) assertEquals(capacity/2, itMid.next()); + assertTrue(isDetached(it0)); + assertTrue(isDetached(itMid)); + assertEquals(capacity, q.size()); + assertEquals(0, q.remainingCapacity()); + } + + /** + * Checks collective sanity of iteration, toArray() and toString(). + */ + public void collectiveSanity() { + boolean fair = rnd.nextBoolean(); + int capacity = rnd.nextInt(10, 20); + ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); + randomizePutIndex(q); + for (int i = 0; i < capacity; i++) { + checkIterationSanity(q); + assertEquals(capacity, q.size() + q.remainingCapacity()); + q.add(i); + } + for (int i = 0; i < (capacity + (capacity >> 1)); i++) { + checkIterationSanity(q); + assertEquals(capacity, q.size() + q.remainingCapacity()); + assertEquals(i, q.peek()); + assertEquals(i, q.poll()); + checkIterationSanity(q); + assertEquals(capacity, q.size() + q.remainingCapacity()); + q.add(capacity + i); + } + for (int i = 0; i < capacity; i++) { + checkIterationSanity(q); + assertEquals(capacity, q.size() + q.remainingCapacity()); + int expected = i + capacity + (capacity >> 1); + assertEquals(expected, q.peek()); + assertEquals(expected, q.poll()); + } + checkIterationSanity(q); + } + + public void iteratorsDetachedWhenExhaustedAndLastRetRemoved() { + boolean fair = rnd.nextBoolean(); + int capacity = rnd.nextInt(2, 10); + ArrayBlockingQueue q = new ArrayBlockingQueue(capacity, fair); + randomizePutIndex(q); + int size = rnd.nextInt(1, capacity + 1); + for (int i = 0; i < size; i++) q.add(i); + Iterator it = q.iterator(); + for (int i = 0; i < size - 1; i++) assertEquals(i, it.next()); + assertEquals(trackedIterators(q), Collections.singletonList(it)); + assertFalse(isDetached(it)); + switch (rnd.nextInt(2)) { + case 0: assertTrue(q.remove(size - 1)); break; + case 1: assertTrue(q.removeIf(e -> e.equals(size - 1))); break; + default: throw new AssertionError(); + } + assertEquals(size - 1, it.next()); // should trigger detach + assertNull(itrs(q)); + assertTrue(isDetached(it)); + assertRemoveHasNoEffect(it, q); + } +} diff --git a/jdk/test/java/util/concurrent/tck/JSR166TestCase.java b/jdk/test/java/util/concurrent/tck/JSR166TestCase.java index 80eb346db4e..1037eb8f3e7 100644 --- a/jdk/test/java/util/concurrent/tck/JSR166TestCase.java +++ b/jdk/test/java/util/concurrent/tck/JSR166TestCase.java @@ -1326,20 +1326,60 @@ public class JSR166TestCase extends TestCase { startTime = System.nanoTime(); else if (millisElapsedSince(startTime) > timeoutMillis) { threadAssertTrue(thread.isAlive()); - return; + fail("timed out waiting for thread to enter wait state"); } Thread.yield(); } } /** - * Waits up to LONG_DELAY_MS for the given thread to enter a wait - * state: BLOCKED, WAITING, or TIMED_WAITING. + * Spin-waits up to the specified number of milliseconds for the given + * thread to enter a wait state: BLOCKED, WAITING, or TIMED_WAITING, + * and additionally satisfy the given condition. + */ + void waitForThreadToEnterWaitState( + Thread thread, long timeoutMillis, Callable waitingForGodot) { + long startTime = 0L; + for (;;) { + Thread.State s = thread.getState(); + if (s == Thread.State.BLOCKED || + s == Thread.State.WAITING || + s == Thread.State.TIMED_WAITING) { + try { + if (waitingForGodot.call()) + return; + } catch (Throwable fail) { threadUnexpectedException(fail); } + } + else if (s == Thread.State.TERMINATED) + fail("Unexpected thread termination"); + else if (startTime == 0L) + startTime = System.nanoTime(); + else if (millisElapsedSince(startTime) > timeoutMillis) { + threadAssertTrue(thread.isAlive()); + fail("timed out waiting for thread to enter wait state"); + } + Thread.yield(); + } + } + + /** + * Spin-waits up to LONG_DELAY_MS milliseconds for the given thread to + * enter a wait state: BLOCKED, WAITING, or TIMED_WAITING. */ void waitForThreadToEnterWaitState(Thread thread) { waitForThreadToEnterWaitState(thread, LONG_DELAY_MS); } + /** + * Spin-waits up to LONG_DELAY_MS milliseconds for the given thread to + * enter a wait state: BLOCKED, WAITING, or TIMED_WAITING, + * and additionally satisfy the given condition. + */ + void waitForThreadToEnterWaitState( + Thread thread, Callable waitingForGodot) { + waitForThreadToEnterWaitState(thread, LONG_DELAY_MS, waitingForGodot); + } + /** * Returns the number of milliseconds since time given by * startNanoTime, which must have been previously returned from a diff --git a/jdk/test/java/util/concurrent/tck/LinkedTransferQueueTest.java b/jdk/test/java/util/concurrent/tck/LinkedTransferQueueTest.java index 55965e6188a..6a22b0e20a9 100644 --- a/jdk/test/java/util/concurrent/tck/LinkedTransferQueueTest.java +++ b/jdk/test/java/util/concurrent/tck/LinkedTransferQueueTest.java @@ -42,6 +42,7 @@ import java.util.List; import java.util.NoSuchElementException; import java.util.Queue; import java.util.concurrent.BlockingQueue; +import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executors; import java.util.concurrent.ExecutorService; @@ -766,9 +767,11 @@ public class LinkedTransferQueueTest extends JSR166TestCase { }}); threadStarted.await(); - waitForThreadToEnterWaitState(t); - assertEquals(1, q.getWaitingConsumerCount()); - assertTrue(q.hasWaitingConsumer()); + Callable oneConsumer + = new Callable() { public Boolean call() { + return q.hasWaitingConsumer() + && q.getWaitingConsumerCount() == 1; }}; + waitForThreadToEnterWaitState(t, oneConsumer); assertTrue(q.offer(one)); assertEquals(0, q.getWaitingConsumerCount()); @@ -790,7 +793,7 @@ public class LinkedTransferQueueTest extends JSR166TestCase { /** * transfer waits until a poll occurs. The transfered element - * is returned by this associated poll. + * is returned by the associated poll. */ public void testTransfer2() throws InterruptedException { final LinkedTransferQueue q = new LinkedTransferQueue<>(); @@ -804,8 +807,11 @@ public class LinkedTransferQueueTest extends JSR166TestCase { }}); threadStarted.await(); - waitForThreadToEnterWaitState(t); - assertEquals(1, q.size()); + Callable oneElement + = new Callable() { public Boolean call() { + return !q.isEmpty() && q.size() == 1; }}; + waitForThreadToEnterWaitState(t, oneElement); + assertSame(five, q.poll()); checkEmpty(q); awaitTermination(t); @@ -868,7 +874,7 @@ public class LinkedTransferQueueTest extends JSR166TestCase { /** * transfer waits until a take occurs. The transfered element - * is returned by this associated take. + * is returned by the associated take. */ public void testTransfer5() throws InterruptedException { final LinkedTransferQueue q = new LinkedTransferQueue<>(); diff --git a/jdk/test/java/util/concurrent/tck/PhaserTest.java b/jdk/test/java/util/concurrent/tck/PhaserTest.java index 1b4d083d73c..832fe3c5ada 100644 --- a/jdk/test/java/util/concurrent/tck/PhaserTest.java +++ b/jdk/test/java/util/concurrent/tck/PhaserTest.java @@ -550,7 +550,7 @@ public class PhaserTest extends JSR166TestCase { }}); await(pleaseArrive); - waitForThreadToEnterWaitState(t, SHORT_DELAY_MS); + waitForThreadToEnterWaitState(t); assertEquals(0, phaser.arrive()); awaitTermination(t); @@ -578,7 +578,7 @@ public class PhaserTest extends JSR166TestCase { }}); await(pleaseArrive); - waitForThreadToEnterWaitState(t, SHORT_DELAY_MS); + waitForThreadToEnterWaitState(t); t.interrupt(); assertEquals(0, phaser.arrive()); awaitTermination(t); @@ -594,20 +594,20 @@ public class PhaserTest extends JSR166TestCase { public void testArriveAndAwaitAdvanceAfterInterrupt() { final Phaser phaser = new Phaser(); assertEquals(0, phaser.register()); - final CountDownLatch pleaseInterrupt = new CountDownLatch(1); + final CountDownLatch pleaseArrive = new CountDownLatch(1); Thread t = newStartedThread(new CheckedRunnable() { public void realRun() { Thread.currentThread().interrupt(); assertEquals(0, phaser.register()); - pleaseInterrupt.countDown(); + pleaseArrive.countDown(); assertTrue(Thread.currentThread().isInterrupted()); assertEquals(1, phaser.arriveAndAwaitAdvance()); - assertTrue(Thread.currentThread().isInterrupted()); + assertTrue(Thread.interrupted()); }}); - await(pleaseInterrupt); - waitForThreadToEnterWaitState(t, SHORT_DELAY_MS); + await(pleaseArrive); + waitForThreadToEnterWaitState(t); Thread.currentThread().interrupt(); assertEquals(1, phaser.arriveAndAwaitAdvance()); assertTrue(Thread.interrupted()); @@ -628,11 +628,11 @@ public class PhaserTest extends JSR166TestCase { assertFalse(Thread.currentThread().isInterrupted()); pleaseInterrupt.countDown(); assertEquals(1, phaser.arriveAndAwaitAdvance()); - assertTrue(Thread.currentThread().isInterrupted()); + assertTrue(Thread.interrupted()); }}); await(pleaseInterrupt); - waitForThreadToEnterWaitState(t, SHORT_DELAY_MS); + waitForThreadToEnterWaitState(t); t.interrupt(); Thread.currentThread().interrupt(); assertEquals(1, phaser.arriveAndAwaitAdvance()); @@ -807,7 +807,7 @@ public class PhaserTest extends JSR166TestCase { assertEquals(THREADS, phaser.getArrivedParties()); assertTrue(millisElapsedSince(startTime) < LONG_DELAY_MS); for (Thread thread : threads) - waitForThreadToEnterWaitState(thread, SHORT_DELAY_MS); + waitForThreadToEnterWaitState(thread); for (Thread thread : threads) assertTrue(thread.isAlive()); assertState(phaser, 0, THREADS + 1, 1); diff --git a/jdk/test/java/util/concurrent/tck/StampedLockTest.java b/jdk/test/java/util/concurrent/tck/StampedLockTest.java index ea0b8f9892b..65ef48d046b 100644 --- a/jdk/test/java/util/concurrent/tck/StampedLockTest.java +++ b/jdk/test/java/util/concurrent/tck/StampedLockTest.java @@ -299,7 +299,6 @@ public class StampedLockTest extends JSR166TestCase { * interruptible operations throw InterruptedException when pre-interrupted */ public void testInterruptibleOperationsThrowInterruptedExceptionWhenPreInterrupted() { - final CountDownLatch running = new CountDownLatch(1); final StampedLock lock = new StampedLock(); Action[] interruptibleLockActions = { @@ -364,7 +363,6 @@ public class StampedLockTest extends JSR166TestCase { * interruptible operations throw InterruptedException when write locked and interrupted */ public void testInterruptibleOperationsThrowInterruptedExceptionWriteLockedInterrupted() { - final CountDownLatch running = new CountDownLatch(1); final StampedLock lock = new StampedLock(); long s = lock.writeLock(); @@ -387,7 +385,6 @@ public class StampedLockTest extends JSR166TestCase { * interruptible operations throw InterruptedException when read locked and interrupted */ public void testInterruptibleOperationsThrowInterruptedExceptionReadLockedInterrupted() { - final CountDownLatch running = new CountDownLatch(1); final StampedLock lock = new StampedLock(); long s = lock.readLock(); @@ -506,29 +503,32 @@ public class StampedLockTest extends JSR166TestCase { } /** - * A writelock succeeds only after a reading thread unlocks + * writeLock() succeeds only after a reading thread unlocks */ public void testWriteAfterReadLock() throws InterruptedException { - final CountDownLatch running = new CountDownLatch(1); + final CountDownLatch aboutToLock = new CountDownLatch(1); final StampedLock lock = new StampedLock(); long rs = lock.readLock(); Thread t = newStartedThread(new CheckedRunnable() { public void realRun() { - running.countDown(); + aboutToLock.countDown(); long s = lock.writeLock(); + assertTrue(lock.isWriteLocked()); + assertFalse(lock.isReadLocked()); lock.unlockWrite(s); }}); - running.await(); - waitForThreadToEnterWaitState(t, MEDIUM_DELAY_MS); + aboutToLock.await(); + waitForThreadToEnterWaitState(t); assertFalse(lock.isWriteLocked()); + assertTrue(lock.isReadLocked()); lock.unlockRead(rs); awaitTermination(t); - assertFalse(lock.isWriteLocked()); + assertUnlocked(lock); } /** - * A writelock succeeds only after reading threads unlock + * writeLock() succeeds only after reading threads unlock */ public void testWriteAfterMultipleReadLocks() { final StampedLock lock = new StampedLock(); @@ -551,35 +551,36 @@ public class StampedLockTest extends JSR166TestCase { assertFalse(lock.isWriteLocked()); lock.unlockRead(s); awaitTermination(t2); - assertFalse(lock.isWriteLocked()); + assertUnlocked(lock); } /** - * Readlocks succeed only after a writing thread unlocks + * readLock() succeed only after a writing thread unlocks */ public void testReadAfterWriteLock() { final StampedLock lock = new StampedLock(); final CountDownLatch threadsStarted = new CountDownLatch(2); final long s = lock.writeLock(); - Thread t1 = newStartedThread(new CheckedRunnable() { + final Runnable acquireReleaseReadLock = new CheckedRunnable() { public void realRun() { threadsStarted.countDown(); long rs = lock.readLock(); + assertTrue(lock.isReadLocked()); + assertFalse(lock.isWriteLocked()); lock.unlockRead(rs); - }}); - Thread t2 = newStartedThread(new CheckedRunnable() { - public void realRun() { - threadsStarted.countDown(); - long rs = lock.readLock(); - lock.unlockRead(rs); - }}); + }}; + Thread t1 = newStartedThread(acquireReleaseReadLock); + Thread t2 = newStartedThread(acquireReleaseReadLock); await(threadsStarted); - waitForThreadToEnterWaitState(t1, MEDIUM_DELAY_MS); - waitForThreadToEnterWaitState(t2, MEDIUM_DELAY_MS); + waitForThreadToEnterWaitState(t1); + waitForThreadToEnterWaitState(t2); + assertTrue(lock.isWriteLocked()); + assertFalse(lock.isReadLocked()); releaseWriteLock(lock, s); awaitTermination(t1); awaitTermination(t2); + assertUnlocked(lock); } /** @@ -765,22 +766,24 @@ public class StampedLockTest extends JSR166TestCase { */ public void testValidateOptimisticWriteLocked2() throws InterruptedException { - final CountDownLatch running = new CountDownLatch(1); + final CountDownLatch locked = new CountDownLatch(1); final StampedLock lock = new StampedLock(); final long p = assertValid(lock, lock.tryOptimisticRead()); Thread t = newStartedThread(new CheckedInterruptedRunnable() { public void realRun() throws InterruptedException { lock.writeLockInterruptibly(); - running.countDown(); + locked.countDown(); lock.writeLockInterruptibly(); }}); - running.await(); + locked.await(); assertFalse(lock.validate(p)); assertEquals(0L, lock.tryOptimisticRead()); + waitForThreadToEnterWaitState(t); t.interrupt(); awaitTermination(t); + assertTrue(lock.isWriteLocked()); } /** From 3d831067ba4b36bf42f987b8e4bbbdbb15e4ba48 Mon Sep 17 00:00:00 2001 From: Igor Ignatyev Date: Fri, 3 Mar 2017 22:00:27 -0800 Subject: [PATCH 04/11] 8176162: com/sun/jndi/dns/Parser.java is not executed Reviewed-by: rriggs --- jdk/test/com/sun/jndi/dns/Parser.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jdk/test/com/sun/jndi/dns/Parser.java b/jdk/test/com/sun/jndi/dns/Parser.java index 58c7bd578b3..7ec2a661fda 100644 --- a/jdk/test/com/sun/jndi/dns/Parser.java +++ b/jdk/test/com/sun/jndi/dns/Parser.java @@ -26,7 +26,9 @@ * @bug 8035105 * @summary DNS resource record parsing * @modules jdk.naming.dns/com.sun.jndi.dns:+open + * * @compile --add-modules jdk.naming.dns Parser.java + * @run main Parser */ import com.sun.jndi.dns.ResourceRecord; From 153f51d64f0a942341954cf866bd2376ce64f3ee Mon Sep 17 00:00:00 2001 From: Amy Lu Date: Mon, 6 Mar 2017 13:43:19 +0800 Subject: [PATCH 05/11] 8176182: 4 security tests are not run Reviewed-by: weijun --- jdk/test/ProblemList.txt | 2 ++ jdk/test/sun/security/ec/SignedObjectChain.java | 5 +++-- jdk/test/sun/security/mscapi/SignedObjectChain.java | 5 +++-- jdk/test/sun/security/rsa/SignedObjectChain.java | 5 +++-- jdk/test/sun/security/ssl/rsa/SignedObjectChain.java | 5 +++-- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/jdk/test/ProblemList.txt b/jdk/test/ProblemList.txt index 09466ea6af0..820caaa3cc0 100644 --- a/jdk/test/ProblemList.txt +++ b/jdk/test/ProblemList.txt @@ -215,6 +215,8 @@ sun/security/tools/jarsigner/warnings/BadKeyUsageTest.java 8026393 generic- javax/net/ssl/DTLS/PacketLossRetransmission.java 8169086 macosx-x64 javax/net/ssl/DTLS/RespondToRetransmit.java 8169086 macosx-x64 +sun/security/mscapi/SignedObjectChain.java 8176183 windows-all + ############################################################################ # jdk_sound diff --git a/jdk/test/sun/security/ec/SignedObjectChain.java b/jdk/test/sun/security/ec/SignedObjectChain.java index 6e93053a776..7c320c0ba27 100644 --- a/jdk/test/sun/security/ec/SignedObjectChain.java +++ b/jdk/test/sun/security/ec/SignedObjectChain.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -24,8 +24,9 @@ /* * @test * @bug 8050374 - * @compile ../../../java/security/SignedObject/Chain.java * @summary Verify a chain of signed objects + * @compile ../../../java/security/SignedObject/Chain.java + * @run main SignedObjectChain */ public class SignedObjectChain { diff --git a/jdk/test/sun/security/mscapi/SignedObjectChain.java b/jdk/test/sun/security/mscapi/SignedObjectChain.java index d436612798f..0c5a4098510 100644 --- a/jdk/test/sun/security/mscapi/SignedObjectChain.java +++ b/jdk/test/sun/security/mscapi/SignedObjectChain.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -24,9 +24,10 @@ /* * @test * @bug 8050374 + * @summary Verify a chain of signed objects * @compile ../../../java/security/SignedObject/Chain.java * @requires os.family == "windows" - * @summary Verify a chain of signed objects + * @run main SignedObjectChain */ public class SignedObjectChain { diff --git a/jdk/test/sun/security/rsa/SignedObjectChain.java b/jdk/test/sun/security/rsa/SignedObjectChain.java index 7bda7cd4df6..cc41d179b1f 100644 --- a/jdk/test/sun/security/rsa/SignedObjectChain.java +++ b/jdk/test/sun/security/rsa/SignedObjectChain.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -24,8 +24,9 @@ /* * @test * @bug 8050374 - * @compile ../../../java/security/SignedObject/Chain.java * @summary Verify a chain of signed objects + * @compile ../../../java/security/SignedObject/Chain.java + * @run main SignedObjectChain */ public class SignedObjectChain { diff --git a/jdk/test/sun/security/ssl/rsa/SignedObjectChain.java b/jdk/test/sun/security/ssl/rsa/SignedObjectChain.java index ba4b00c56d8..212384ac5bd 100644 --- a/jdk/test/sun/security/ssl/rsa/SignedObjectChain.java +++ b/jdk/test/sun/security/ssl/rsa/SignedObjectChain.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -24,8 +24,9 @@ /* * @test * @bug 8050374 - * @compile ../../../../java/security/SignedObject/Chain.java * @summary Verify a chain of signed objects + * @compile ../../../../java/security/SignedObject/Chain.java + * @run main SignedObjectChain */ public class SignedObjectChain { From b7c3f6ef9492acfc76500a3c869ab8fbd3e5261f Mon Sep 17 00:00:00 2001 From: Amy Lu Date: Mon, 6 Mar 2017 15:58:17 +0800 Subject: [PATCH 06/11] 8176187: jdk/internal/misc/JavaLangAccess/NewUnsafeString.java is not run Reviewed-by: alanb --- jdk/test/ProblemList.txt | 2 ++ jdk/test/jdk/internal/misc/JavaLangAccess/NewUnsafeString.java | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/jdk/test/ProblemList.txt b/jdk/test/ProblemList.txt index 820caaa3cc0..0b5aa8f4372 100644 --- a/jdk/test/ProblemList.txt +++ b/jdk/test/ProblemList.txt @@ -125,6 +125,8 @@ java/beans/Introspector/8132566/OverrideUserDefPropertyInfoTest.java 8132565 gen java/lang/StringCoding/CheckEncodings.sh 7008363 generic-all +jdk/internal/misc/JavaLangAccess/NewUnsafeString.java 8176188 generic-all + ############################################################################ # jdk_instrument diff --git a/jdk/test/jdk/internal/misc/JavaLangAccess/NewUnsafeString.java b/jdk/test/jdk/internal/misc/JavaLangAccess/NewUnsafeString.java index 50235107153..302fb185809 100644 --- a/jdk/test/jdk/internal/misc/JavaLangAccess/NewUnsafeString.java +++ b/jdk/test/jdk/internal/misc/JavaLangAccess/NewUnsafeString.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 2017, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -32,6 +32,7 @@ import jdk.internal.misc.SharedSecrets; * @summary Test JavaLangAccess.newUnsafeString * @modules java.base/jdk.internal.misc * @compile -XDignore.symbol.file NewUnsafeString.java + * @run main NewUnsafeString */ public class NewUnsafeString { From 7fa43a44b4c96bacd1f6051fb258e07dc81d9559 Mon Sep 17 00:00:00 2001 From: Amy Lu Date: Mon, 6 Mar 2017 16:07:50 +0800 Subject: [PATCH 07/11] 8176185: java/util/TimeZone/UTCAliasTest.java is not run Reviewed-by: alanb --- jdk/test/java/util/TimeZone/UTCAliasTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jdk/test/java/util/TimeZone/UTCAliasTest.java b/jdk/test/java/util/TimeZone/UTCAliasTest.java index aa7079326c0..ffe7b63fd71 100644 --- a/jdk/test/java/util/TimeZone/UTCAliasTest.java +++ b/jdk/test/java/util/TimeZone/UTCAliasTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2017, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -27,6 +27,7 @@ * @summary Make sure that "UTC" is an alias of "Etc/UTC" as defined in the tzdata backward. * @modules java.base/sun.util.calendar * @compile -XDignore.symbol.file UTCAliasTest.java + * @run main UTCAliasTest */ import java.util.*; From 097a0b8a955412998388e10584f456e2eee4e134 Mon Sep 17 00:00:00 2001 From: Naoto Sato Date: Mon, 6 Mar 2017 18:54:53 -0800 Subject: [PATCH 08/11] 8174736: [JCP] [Mac]Cannot launch JCP on Mac os with language set to "Chinese, Simplified" while region is not China Reviewed-by: bchristi --- .../macosx/native/libjava/java_props_macosx.c | 84 ++++++++++--------- 1 file changed, 45 insertions(+), 39 deletions(-) diff --git a/jdk/src/java.base/macosx/native/libjava/java_props_macosx.c b/jdk/src/java.base/macosx/native/libjava/java_props_macosx.c index 8246b8130a5..b11a4d00143 100644 --- a/jdk/src/java.base/macosx/native/libjava/java_props_macosx.c +++ b/jdk/src/java.base/macosx/native/libjava/java_props_macosx.c @@ -46,6 +46,8 @@ char *getPosixLocale(int cat) { #define LOCALEIDLENGTH 128 char *getMacOSXLocale(int cat) { + const char* retVal = NULL; + switch (cat) { case LC_MESSAGES: { @@ -72,41 +74,7 @@ char *getMacOSXLocale(int cat) { } CFRelease(languages); - // Language IDs use the language designators and (optional) region - // and script designators of BCP 47. So possible formats are: - // - // "en" (language designator only) - // "haw" (3-letter lanuage designator) - // "en-GB" (language with alpha-2 region designator) - // "es-419" (language with 3-digit UN M.49 area code) - // "zh-Hans" (language with ISO 15924 script designator) - // "zh-Hans-US" (language with ISO 15924 script designator and region) - // "zh-Hans-419" (language with ISO 15924 script designator and UN M.49) - // - // In the case of region designators (alpha-2 and/or UN M.49), we convert - // to our locale string format by changing '-' to '_'. That is, if - // the '-' is followed by fewer than 4 chars. - char* scriptOrRegion = strchr(languageString, '-'); - if (scriptOrRegion != NULL) { - int length = strlen(scriptOrRegion); - if (length > 5) { - // Region and script both exist. Honor the script for now - scriptOrRegion[5] = '\0'; - } else if (length < 5) { - *scriptOrRegion = '_'; - - assert((length == 3 && - // '-' followed by a 2 character region designator - isalpha(scriptOrRegion[1]) && - isalpha(scriptOrRegion[2])) || - (length == 4 && - // '-' followed by a 3-digit UN M.49 area code - isdigit(scriptOrRegion[1]) && - isdigit(scriptOrRegion[2]) && - isdigit(scriptOrRegion[3]))); - } - } - const char* retVal = languageString; + retVal = languageString; // Special case for Portuguese in Brazil: // The language code needs the "_BR" region code (to distinguish it @@ -120,20 +88,58 @@ char *getMacOSXLocale(int cat) { strcmp(localeString, "pt_BR") == 0) { retVal = localeString; } - return strdup(retVal); } break; default: { char localeString[LOCALEIDLENGTH]; - if (CFStringGetCString(CFLocaleGetIdentifier(CFLocaleCopyCurrent()), - localeString, LOCALEIDLENGTH, CFStringGetSystemEncoding())) { - return strdup(localeString); + if (!CFStringGetCString(CFLocaleGetIdentifier(CFLocaleCopyCurrent()), + localeString, LOCALEIDLENGTH, CFStringGetSystemEncoding())) { + return NULL; } + retVal = localeString; } break; } + if (retVal != NULL) { + // Language IDs use the language designators and (optional) region + // and script designators of BCP 47. So possible formats are: + // + // "en" (language designator only) + // "haw" (3-letter lanuage designator) + // "en-GB" (language with alpha-2 region designator) + // "es-419" (language with 3-digit UN M.49 area code) + // "zh-Hans" (language with ISO 15924 script designator) + // "zh-Hans-US" (language with ISO 15924 script designator and region) + // "zh-Hans-419" (language with ISO 15924 script designator and UN M.49) + // + // In the case of region designators (alpha-2 and/or UN M.49), we convert + // to our locale string format by changing '-' to '_'. That is, if + // the '-' is followed by fewer than 4 chars. + char* scriptOrRegion = strchr(retVal, '-'); + if (scriptOrRegion != NULL) { + int length = strlen(scriptOrRegion); + if (length > 5) { + // Region and script both exist. Honor the script for now + scriptOrRegion[5] = '\0'; + } else if (length < 5) { + *scriptOrRegion = '_'; + + assert((length == 3 && + // '-' followed by a 2 character region designator + isalpha(scriptOrRegion[1]) && + isalpha(scriptOrRegion[2])) || + (length == 4 && + // '-' followed by a 3-digit UN M.49 area code + isdigit(scriptOrRegion[1]) && + isdigit(scriptOrRegion[2]) && + isdigit(scriptOrRegion[3]))); + } + } + + return strdup(retVal); + } return NULL; } From 080a88360a8528be241da9eeaef3e2f368b59337 Mon Sep 17 00:00:00 2001 From: Sergei Kovalev Date: Tue, 7 Mar 2017 16:02:20 +0300 Subject: [PATCH 09/11] 8176213: 78 sun/security/krb5/auto tests failing due to undeclared dependecies Reviewed-by: weijun --- jdk/test/ProblemList.txt | 2 ++ .../sun/security/krb5/auto/HttpNegotiateServer.java | 11 ++++++++++- jdk/test/sun/security/krb5/auto/Renew.java | 8 +------- jdk/test/sun/security/krb5/auto/TEST.properties | 4 +++- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/jdk/test/ProblemList.txt b/jdk/test/ProblemList.txt index 0b5aa8f4372..9ade1eca7b8 100644 --- a/jdk/test/ProblemList.txt +++ b/jdk/test/ProblemList.txt @@ -219,6 +219,8 @@ javax/net/ssl/DTLS/RespondToRetransmit.java 8169086 macosx-x sun/security/mscapi/SignedObjectChain.java 8176183 windows-all +sun/security/krb5/auto/Basic.java 8176296 generic-all + ############################################################################ # jdk_sound diff --git a/jdk/test/sun/security/krb5/auto/HttpNegotiateServer.java b/jdk/test/sun/security/krb5/auto/HttpNegotiateServer.java index 55f2d1a4d15..38d9910893b 100644 --- a/jdk/test/sun/security/krb5/auto/HttpNegotiateServer.java +++ b/jdk/test/sun/security/krb5/auto/HttpNegotiateServer.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -24,6 +24,15 @@ /* * @test * @bug 6578647 6829283 8171340 + * @modules java.base/sun.security.util + * java.security.jgss/sun.security.krb5.internal:+open + * java.security.jgss/sun.security.jgss + * java.security.jgss/sun.security.krb5:+open + * java.security.jgss/sun.security.krb5.internal.crypto + * java.security.jgss/sun.security.krb5.internal.ktab + * jdk.security.auth + * jdk.security.jgss + * jdk.httpserver * @run main/othervm HttpNegotiateServer * @summary Undefined requesting URL in java.net.Authenticator * .getPasswordAuthentication() diff --git a/jdk/test/sun/security/krb5/auto/Renew.java b/jdk/test/sun/security/krb5/auto/Renew.java index 41e3fcde6b5..0c441496d93 100644 --- a/jdk/test/sun/security/krb5/auto/Renew.java +++ b/jdk/test/sun/security/krb5/auto/Renew.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2016 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2017 Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -26,12 +26,6 @@ * @bug 8058290 * @summary JAAS Krb5LoginModule has suspect ticket-renewal logic, * relies on clockskew grace - * @modules java.base/sun.security.util - * java.security.jgss/sun.security.krb5:+open - * java.security.jgss/sun.security.krb5.internal:+open - * java.security.jgss/sun.security.krb5.internal.ccache - * java.security.jgss/sun.security.krb5.internal.crypto - * java.security.jgss/sun.security.krb5.internal.ktab * @compile -XDignore.symbol.file Renew.java * @run main/othervm Renew 1 * @run main/othervm Renew 2 diff --git a/jdk/test/sun/security/krb5/auto/TEST.properties b/jdk/test/sun/security/krb5/auto/TEST.properties index 777c20d8233..fa23fd6fb0d 100644 --- a/jdk/test/sun/security/krb5/auto/TEST.properties +++ b/jdk/test/sun/security/krb5/auto/TEST.properties @@ -6,4 +6,6 @@ modules java.base/jdk.internal.misc \ java.security.jgss/sun.security.krb5.internal.ccache \ java.security.jgss/sun.security.krb5.internal.rcache \ java.security.jgss/sun.security.krb5.internal.crypto \ - java.security.jgss/sun.security.krb5.internal.ktab + java.security.jgss/sun.security.krb5.internal.ktab \ + jdk.security.auth \ + jdk.security.jgss From 6b3143e8313508a4f54f3a2e952429a9b59a6170 Mon Sep 17 00:00:00 2001 From: Weijun Wang Date: Tue, 7 Mar 2017 22:42:11 +0800 Subject: [PATCH 10/11] 8171319: keytool should print out warnings when reading or generating cert/cert req using weak algorithms Reviewed-by: coffeys --- .../classes/sun/security/pkcs10/PKCS10.java | 10 +- .../provider/certpath/BasicChecker.java | 12 +- .../sun/security/tools/keytool/Main.java | 337 ++++++++--- .../sun/security/tools/keytool/Resources.java | 35 +- .../sun/security/x509/X509CRLImpl.java | 7 +- .../tools/jarsigner/TimestampCheck.java | 14 + .../sun/security/tools/keytool/WeakAlg.java | 557 ++++++++++++++++++ 7 files changed, 891 insertions(+), 81 deletions(-) create mode 100644 jdk/test/sun/security/tools/keytool/WeakAlg.java diff --git a/jdk/src/java.base/share/classes/sun/security/pkcs10/PKCS10.java b/jdk/src/java.base/share/classes/sun/security/pkcs10/PKCS10.java index 02ed730b9af..9302580db1c 100644 --- a/jdk/src/java.base/share/classes/sun/security/pkcs10/PKCS10.java +++ b/jdk/src/java.base/share/classes/sun/security/pkcs10/PKCS10.java @@ -167,7 +167,8 @@ public class PKCS10 { // key and signature algorithm we found. // try { - sig = Signature.getInstance(id.getName()); + sigAlg = id.getName(); + sig = Signature.getInstance(sigAlg); sig.initVerify(subjectPublicKeyInfo); sig.update(data); if (!sig.verify(sigData)) @@ -218,6 +219,7 @@ public class PKCS10 { signature.update(certificateRequestInfo, 0, certificateRequestInfo.length); sig = signature.sign(); + sigAlg = signature.getAlgorithm(); /* * Build guts of SIGNED macro @@ -250,6 +252,11 @@ public class PKCS10 { public PublicKey getSubjectPublicKeyInfo() { return subjectPublicKeyInfo; } + /** + * Returns the signature algorithm. + */ + public String getSigAlg() { return sigAlg; } + /** * Returns the additional attributes requested. */ @@ -348,6 +355,7 @@ public class PKCS10 { private X500Name subject; private PublicKey subjectPublicKeyInfo; + private String sigAlg; private PKCS10Attributes attributeSet; private byte[] encoded; // signed } diff --git a/jdk/src/java.base/share/classes/sun/security/provider/certpath/BasicChecker.java b/jdk/src/java.base/share/classes/sun/security/provider/certpath/BasicChecker.java index 49a0368753e..43ff7b7cd9d 100644 --- a/jdk/src/java.base/share/classes/sun/security/provider/certpath/BasicChecker.java +++ b/jdk/src/java.base/share/classes/sun/security/provider/certpath/BasicChecker.java @@ -51,7 +51,7 @@ import sun.security.util.Debug; /** * BasicChecker is a PKIXCertPathChecker that checks the basic information - * on a PKIX certificate, namely the signature, timestamp, and subject/issuer + * on a PKIX certificate, namely the signature, validity, and subject/issuer * name chaining. * * @since 1.4 @@ -125,7 +125,7 @@ class BasicChecker extends PKIXCertPathChecker { } /** - * Performs the signature, timestamp, and subject/issuer name chaining + * Performs the signature, validity, and subject/issuer name chaining * checks on the certificate using its internal state. This method does * not remove any critical extensions from the Collection. * @@ -141,7 +141,7 @@ class BasicChecker extends PKIXCertPathChecker { X509Certificate currCert = (X509Certificate)cert; if (!sigOnly) { - verifyTimestamp(currCert); + verifyValidity(currCert); verifyNameChaining(currCert); } verifySignature(currCert); @@ -177,12 +177,12 @@ class BasicChecker extends PKIXCertPathChecker { } /** - * Internal method to verify the timestamp on a certificate + * Internal method to verify the validity on a certificate */ - private void verifyTimestamp(X509Certificate cert) + private void verifyValidity(X509Certificate cert) throws CertPathValidatorException { - String msg = "timestamp"; + String msg = "validity"; if (debug != null) debug.println("---checking " + msg + ":" + date.toString() + "..."); diff --git a/jdk/src/java.base/share/classes/sun/security/tools/keytool/Main.java b/jdk/src/java.base/share/classes/sun/security/tools/keytool/Main.java index b73773f5711..c71df1a2cbe 100644 --- a/jdk/src/java.base/share/classes/sun/security/tools/keytool/Main.java +++ b/jdk/src/java.base/share/classes/sun/security/tools/keytool/Main.java @@ -27,6 +27,7 @@ package sun.security.tools.keytool; import java.io.*; import java.security.CodeSigner; +import java.security.CryptoPrimitive; import java.security.KeyStore; import java.security.KeyStoreException; import java.security.MessageDigest; @@ -156,6 +157,7 @@ public final class Main { private boolean protectedPath = false; private boolean srcprotectedPath = false; private boolean cacerts = false; + private boolean nowarn = false; private CertificateFactory cf = null; private KeyStore caks = null; // "cacerts" keystore private char[] srcstorePass = null; @@ -166,6 +168,16 @@ public final class Main { private List ids = new ArrayList<>(); // used in GENCRL private List v3ext = new ArrayList<>(); + // Warnings on weak algorithms + private List weakWarnings = new ArrayList<>(); + + private static final DisabledAlgorithmConstraints DISABLED_CHECK = + new DisabledAlgorithmConstraints( + DisabledAlgorithmConstraints.PROPERTY_CERTPATH_DISABLED_ALGS); + + private static final Set SIG_PRIMITIVE_SET = Collections + .unmodifiableSet(EnumSet.of(CryptoPrimitive.SIGNATURE)); + enum Command { CERTREQ("Generates.a.certificate.request", ALIAS, SIGALG, FILEOUT, KEYPASS, KEYSTORE, DNAME, @@ -351,7 +363,7 @@ public final class Main { private static final String NONE = "NONE"; private static final String P11KEYSTORE = "PKCS11"; private static final String P12KEYSTORE = "PKCS12"; - private final String keyAlias = "mykey"; + private static final String keyAlias = "mykey"; // for i18n private static final java.util.ResourceBundle rb = @@ -387,6 +399,7 @@ public final class Main { throw e; } } finally { + printWeakWarnings(false); for (char[] pass : passwords) { if (pass != null) { Arrays.fill(pass, ' '); @@ -476,6 +489,8 @@ public final class Main { help = true; } else if (collator.compare(flags, "-conf") == 0) { i++; + } else if (collator.compare(flags, "-nowarn") == 0) { + nowarn = true; } else if (collator.compare(flags, "-keystore") == 0) { ksfname = args[++i]; if (new File(ksfname).getCanonicalPath().equals( @@ -1152,11 +1167,11 @@ public final class Main { } else if (command == LIST) { if (storePass == null && !KeyStoreUtil.isWindowsKeyStore(storetype)) { - printWarning(); + printNoIntegrityWarning(); } if (alias != null) { - doPrintEntry(alias, out); + doPrintEntry(rb.getString("the.certificate"), alias, out); } else { doPrintEntries(out); } @@ -1253,6 +1268,12 @@ public final class Main { throws Exception { + if (keyStore.containsAlias(alias) == false) { + MessageFormat form = new MessageFormat + (rb.getString("Alias.alias.does.not.exist")); + Object[] source = {alias}; + throw new Exception(form.format(source)); + } Certificate signerCert = keyStore.getCertificate(alias); byte[] encoded = signerCert.getEncoded(); X509CertImpl signerCertImpl = new X509CertImpl(encoded); @@ -1306,6 +1327,8 @@ public final class Main { byte[] rawReq = Pem.decode(new String(sb)); PKCS10 req = new PKCS10(rawReq); + checkWeak(rb.getString("the.certificate.request"), req); + info.set(X509CertInfo.KEY, new CertificateX509Key(req.getSubjectPublicKeyInfo())); info.set(X509CertInfo.SUBJECT, dname==null?req.getSubjectName():new X500Name(dname)); @@ -1335,6 +1358,9 @@ public final class Main { } } } + + checkWeak(rb.getString("the.issuer"), keyStore.getCertificateChain(alias)); + checkWeak(rb.getString("the.generated.certificate"), cert); } private void doGenCRL(PrintStream out) @@ -1385,6 +1411,7 @@ public final class Main { } else { out.write(crl.getEncodedInternal()); } + checkWeak(rb.getString("the.generated.crl"), crl, privateKey); } /** @@ -1431,6 +1458,8 @@ public final class Main { // Sign the request and base-64 encode it request.encodeAndSign(subject, signature); request.print(out); + + checkWeak(rb.getString("the.generated.certificate.request"), request); } /** @@ -1454,7 +1483,7 @@ public final class Main { { if (storePass == null && !KeyStoreUtil.isWindowsKeyStore(storetype)) { - printWarning(); + printNoIntegrityWarning(); } if (alias == null) { alias = keyAlias; @@ -1474,6 +1503,7 @@ public final class Main { throw new Exception(form.format(source)); } dumpCert(cert, out); + checkWeak(rb.getString("the.certificate"), cert); } /** @@ -1729,6 +1759,8 @@ public final class Main { keyPass = promptForKeyPass(alias, null, storePass); } keyStore.setKeyEntry(alias, privKey, keyPass, chain); + + checkWeak(rb.getString("the.generated.certificate"), chain[0]); } /** @@ -1810,7 +1842,7 @@ public final class Main { /** * Prints a single keystore entry. */ - private void doPrintEntry(String alias, PrintStream out) + private void doPrintEntry(String label, String alias, PrintStream out) throws Exception { if (keyStore.containsAlias(alias) == false) { @@ -1881,12 +1913,14 @@ public final class Main { } else { dumpCert(chain[i], out); } + checkWeak(label, chain[i]); } } else { // Print the digest of the user cert only out.println (rb.getString("Certificate.fingerprint.SHA.256.") + getCertFingerPrint("SHA-256", chain[0])); + checkWeak(label, chain); } } } else if (keyStore.entryInstanceOf(alias, @@ -1909,6 +1943,7 @@ public final class Main { out.println(rb.getString("Certificate.fingerprint.SHA.256.") + getCertFingerPrint("SHA-256", cert)); } + checkWeak(label, cert); } else { out.println(rb.getString("Unknown.Entry.Type")); } @@ -1992,7 +2027,7 @@ public final class Main { if (srcstorePass == null && !KeyStoreUtil.isWindowsKeyStore(srcstoretype)) { - // anti refactoring, copied from printWarning(), + // anti refactoring, copied from printNoIntegrityWarning(), // but change 2 lines System.err.println(); System.err.println(rb.getString @@ -2092,6 +2127,10 @@ public final class Main { "The.destination.pkcs12.keystore.has.different.storepass.and.keypass.Please.retry.with.destkeypass.specified.")); } } + Certificate c = srckeystore.getCertificate(alias); + if (c != null) { + checkWeak("<" + newAlias + ">", c); + } return 1; } catch (KeyStoreException kse) { Object[] source2 = {alias, kse.toString()}; @@ -2154,7 +2193,7 @@ public final class Main { for (Enumeration e = keyStore.aliases(); e.hasMoreElements(); ) { String alias = e.nextElement(); - doPrintEntry(alias, out); + doPrintEntry("<" + alias + ">", alias, out); if (verbose || rfc) { out.println(rb.getString("NEWLINE")); out.println(rb.getString @@ -2300,19 +2339,28 @@ public final class Main { for (CRL crl: loadCRLs(src)) { printCRL(crl, out); String issuer = null; + Certificate signer = null; if (caks != null) { issuer = verifyCRL(caks, crl); if (issuer != null) { + signer = caks.getCertificate(issuer); out.printf(rb.getString( - "verified.by.s.in.s"), issuer, "cacerts"); + "verified.by.s.in.s.weak"), + issuer, + "cacerts", + withWeak(signer.getPublicKey())); out.println(); } } if (issuer == null && keyStore != null) { issuer = verifyCRL(keyStore, crl); if (issuer != null) { + signer = keyStore.getCertificate(issuer); out.printf(rb.getString( - "verified.by.s.in.s"), issuer, "keystore"); + "verified.by.s.in.s.weak"), + issuer, + "keystore", + withWeak(signer.getPublicKey())); out.println(); } } @@ -2324,18 +2372,26 @@ public final class Main { out.println(rb.getString ("STARNN")); } + checkWeak(rb.getString("the.crl"), crl, signer == null ? null : signer.getPublicKey()); } } private void printCRL(CRL crl, PrintStream out) throws Exception { + X509CRL xcrl = (X509CRL)crl; if (rfc) { - X509CRL xcrl = (X509CRL)crl; out.println("-----BEGIN X509 CRL-----"); out.println(Base64.getMimeEncoder(64, CRLF).encodeToString(xcrl.getEncoded())); out.println("-----END X509 CRL-----"); } else { - out.println(crl.toString()); + String s; + if (crl instanceof X509CRLImpl) { + X509CRLImpl x509crl = (X509CRLImpl) crl; + s = x509crl.toStringWithAlgName(withWeak("" + x509crl.getSigAlgId())); + } else { + s = crl.toString(); + } + out.println(s); } } @@ -2362,8 +2418,11 @@ public final class Main { PKCS10 req = new PKCS10(Pem.decode(new String(sb))); PublicKey pkey = req.getSubjectPublicKeyInfo(); - out.printf(rb.getString("PKCS.10.Certificate.Request.Version.1.0.Subject.s.Public.Key.s.format.s.key."), - req.getSubjectName(), pkey.getFormat(), pkey.getAlgorithm()); + out.printf(rb.getString("PKCS.10.with.weak"), + req.getSubjectName(), + pkey.getFormat(), + withWeak(pkey), + withWeak(req.getSigAlg())); for (PKCS10Attribute attr: req.getAttributes().getAttributes()) { ObjectIdentifier oid = attr.getAttributeId(); if (oid.equals(PKCS9Attribute.EXTENSION_REQUEST_OID)) { @@ -2386,6 +2445,7 @@ public final class Main { if (debug) { out.println(req); // Just to see more, say, public key length... } + checkWeak(rb.getString("the.certificate.request"), req); } /** @@ -2425,6 +2485,15 @@ public final class Main { if (i < (certs.length-1)) { out.println(); } + checkWeak(oneInMany(rb.getString("the.certificate"), i, certs.length), x509Cert); + } + } + + private static String oneInMany(String label, int i, int num) { + if (num == 1) { + return label; + } else { + return String.format(rb.getString("one.in.many"), label, i+1, num); } } @@ -2458,7 +2527,11 @@ public final class Main { out.println(); out.println(rb.getString("Signature.")); out.println(); - for (Certificate cert: signer.getSignerCertPath().getCertificates()) { + + List certs + = signer.getSignerCertPath().getCertificates(); + int cc = 0; + for (Certificate cert: certs) { X509Certificate x = (X509Certificate)cert; if (rfc) { out.println(rb.getString("Certificate.owner.") + x.getSubjectDN() + "\n"); @@ -2467,12 +2540,15 @@ public final class Main { printX509Cert(x, out); } out.println(); + checkWeak(oneInMany(rb.getString("the.certificate"), cc++, certs.size()), x); } Timestamp ts = signer.getTimestamp(); if (ts != null) { out.println(rb.getString("Timestamp.")); out.println(); - for (Certificate cert: ts.getSignerCertPath().getCertificates()) { + certs = ts.getSignerCertPath().getCertificates(); + cc = 0; + for (Certificate cert: certs) { X509Certificate x = (X509Certificate)cert; if (rfc) { out.println(rb.getString("Certificate.owner.") + x.getSubjectDN() + "\n"); @@ -2481,6 +2557,7 @@ public final class Main { printX509Cert(x, out); } out.println(); + checkWeak(oneInMany(rb.getString("the.tsa.certificate"), cc++, certs.size()), x); } } } @@ -2523,6 +2600,7 @@ public final class Main { printX509Cert((X509Certificate)cert, out); out.println(); } + checkWeak(oneInMany(rb.getString("the.certificate"), i, chain.size()), cert); } catch (Exception e) { if (debug) { e.printStackTrace(); @@ -2698,7 +2776,7 @@ public final class Main { } // Now store the newly established chain in the keystore. The new - // chain replaces the old one. + // chain replaces the old one. The chain can be null if user chooses no. if (newChain != null) { keyStore.setKeyEntry(alias, privKey, (keyPass != null) ? keyPass : storePass, @@ -2735,6 +2813,12 @@ public final class Main { throw new Exception(rb.getString("Input.not.an.X.509.certificate")); } + if (noprompt) { + keyStore.setCertificateEntry(alias, cert); + checkWeak(rb.getString("the.input"), cert); + return true; + } + // if certificate is self-signed, make sure it verifies boolean selfSigned = false; if (KeyStoreUtil.isSelfSigned(cert)) { @@ -2742,11 +2826,6 @@ public final class Main { selfSigned = true; } - if (noprompt) { - keyStore.setCertificateEntry(alias, cert); - return true; - } - // check if cert already exists in keystore String reply = null; String trustalias = keyStore.getCertificateAlias(cert); @@ -2755,6 +2834,8 @@ public final class Main { ("Certificate.already.exists.in.keystore.under.alias.trustalias.")); Object[] source = {trustalias}; System.err.println(form.format(source)); + checkWeak(rb.getString("the.input"), cert); + printWeakWarnings(true); reply = getYesNoReply (rb.getString("Do.you.still.want.to.add.it.no.")); } else if (selfSigned) { @@ -2764,6 +2845,8 @@ public final class Main { ("Certificate.already.exists.in.system.wide.CA.keystore.under.alias.trustalias.")); Object[] source = {trustalias}; System.err.println(form.format(source)); + checkWeak(rb.getString("the.input"), cert); + printWeakWarnings(true); reply = getYesNoReply (rb.getString("Do.you.still.want.to.add.it.to.your.own.keystore.no.")); } @@ -2771,6 +2854,8 @@ public final class Main { // Print the cert and ask user if they really want to add // it to their keystore printX509Cert(cert, System.out); + checkWeak(rb.getString("the.input"), cert); + printWeakWarnings(true); reply = getYesNoReply (rb.getString("Trust.this.certificate.no.")); } @@ -2784,6 +2869,7 @@ public final class Main { } } + // Not found in this keystore and not self-signed // Try to establish trust chain try { Certificate[] chain = establishCertChain(null, cert); @@ -2795,6 +2881,8 @@ public final class Main { // Print the cert and ask user if they really want to add it to // their keystore printX509Cert(cert, System.out); + checkWeak(rb.getString("the.input"), cert); + printWeakWarnings(true); reply = getYesNoReply (rb.getString("Trust.this.certificate.no.")); if ("YES".equals(reply)) { @@ -2933,6 +3021,24 @@ public final class Main { return keyPass; } + private String withWeak(String alg) { + if (DISABLED_CHECK.permits(SIG_PRIMITIVE_SET, alg, null)) { + return alg; + } else { + return String.format(rb.getString("with.weak"), alg); + } + } + + private String withWeak(PublicKey key) { + if (DISABLED_CHECK.permits(SIG_PRIMITIVE_SET, key)) { + return String.format(rb.getString("key.bit"), + KeyUtil.getKeySize(key), key.getAlgorithm()); + } else { + return String.format(rb.getString("key.bit.weak"), + KeyUtil.getKeySize(key), key.getAlgorithm()); + } + } + /** * Prints a certificate in a human readable format. */ @@ -2941,7 +3047,7 @@ public final class Main { { MessageFormat form = new MessageFormat - (rb.getString(".PATTERN.printX509Cert")); + (rb.getString(".PATTERN.printX509Cert.with.weak")); PublicKey pkey = cert.getPublicKey(); Object[] source = {cert.getSubjectDN().toString(), cert.getIssuerDN().toString(), @@ -2950,10 +3056,9 @@ public final class Main { cert.getNotAfter().toString(), getCertFingerPrint("SHA-1", cert), getCertFingerPrint("SHA-256", cert), - cert.getSigAlgName(), - pkey.getAlgorithm(), - KeyUtil.getKeySize(pkey), - cert.getVersion(), + withWeak(cert.getSigAlgName()), + withWeak(pkey), + cert.getVersion() }; out.println(form.format(source)); @@ -3003,12 +3108,12 @@ public final class Main { * @param ks the keystore to search with, not null * @return cert itself if it's already inside ks, * or a certificate inside ks who signs cert, - * or null otherwise. + * or null otherwise. A label is added. */ - private static Certificate getTrustedSigner(Certificate cert, KeyStore ks) - throws Exception { + private static Pair + getTrustedSigner(Certificate cert, KeyStore ks) throws Exception { if (ks.getCertificateAlias(cert) != null) { - return cert; + return new Pair<>("", cert); } for (Enumeration aliases = ks.aliases(); aliases.hasMoreElements(); ) { @@ -3017,7 +3122,7 @@ public final class Main { if (trustedCert != null) { try { cert.verify(trustedCert.getPublicKey()); - return trustedCert; + return new Pair<>(name, trustedCert); } catch (Exception e) { // Not verified, skip to the next one } @@ -3281,7 +3386,7 @@ public final class Main { /** * Prints warning about missing integrity check. */ - private void printWarning() { + private void printNoIntegrityWarning() { System.err.println(); System.err.println(rb.getString (".WARNING.WARNING.WARNING.")); @@ -3306,6 +3411,9 @@ public final class Main { Certificate[] replyCerts) throws Exception { + + checkWeak(rb.getString("reply"), replyCerts); + // order the certs in the reply (bottom-up). // we know that all certs in the reply are of type X.509, because // we parsed them using an X.509 certificate factory @@ -3358,9 +3466,11 @@ public final class Main { // do we trust the cert at the top? Certificate topCert = replyCerts[replyCerts.length-1]; - Certificate root = getTrustedSigner(topCert, keyStore); + boolean fromKeyStore = true; + Pair root = getTrustedSigner(topCert, keyStore); if (root == null && trustcacerts && caks != null) { root = getTrustedSigner(topCert, caks); + fromKeyStore = false; } if (root == null) { System.err.println(); @@ -3369,33 +3479,42 @@ public final class Main { printX509Cert((X509Certificate)topCert, System.out); System.err.println(); System.err.print(rb.getString(".is.not.trusted.")); + printWeakWarnings(true); String reply = getYesNoReply (rb.getString("Install.reply.anyway.no.")); if ("NO".equals(reply)) { return null; } } else { - if (root != topCert) { + if (root.snd != topCert) { // append the root CA cert to the chain Certificate[] tmpCerts = new Certificate[replyCerts.length+1]; System.arraycopy(replyCerts, 0, tmpCerts, 0, replyCerts.length); - tmpCerts[tmpCerts.length-1] = root; + tmpCerts[tmpCerts.length-1] = root.snd; replyCerts = tmpCerts; + checkWeak(String.format(rb.getString(fromKeyStore ? + "alias.in.keystore" : + "alias.in.cacerts"), + root.fst), + root.snd); } } - return replyCerts; } /** * Establishes a certificate chain (using trusted certificates in the - * keystore), starting with the user certificate + * keystore and cacerts), starting with the reply (certToVerify) * and ending at a self-signed certificate found in the keystore. * - * @param userCert the user certificate of the alias - * @param certToVerify the single certificate provided in the reply + * @param userCert optional existing certificate, mostly likely be the + * original self-signed cert created by -genkeypair. + * It must have the same public key as certToVerify + * but cannot be the same cert. + * @param certToVerify the starting certificate to build the chain + * @returns the established chain, might be null if user decides not */ private Certificate[] establishCertChain(Certificate userCert, Certificate certToVerify) @@ -3423,30 +3542,37 @@ public final class Main { // Use the subject distinguished name as the key into the hash table. // All certificates associated with the same subject distinguished // name are stored in the same hash table entry as a vector. - Hashtable> certs = null; + Hashtable>> certs = null; if (keyStore.size() > 0) { - certs = new Hashtable>(11); + certs = new Hashtable<>(11); keystorecerts2Hashtable(keyStore, certs); } if (trustcacerts) { if (caks!=null && caks.size()>0) { if (certs == null) { - certs = new Hashtable>(11); + certs = new Hashtable<>(11); } keystorecerts2Hashtable(caks, certs); } } // start building chain - Vector chain = new Vector<>(2); - if (buildChain((X509Certificate)certToVerify, chain, certs)) { - Certificate[] newChain = new Certificate[chain.size()]; + Vector> chain = new Vector<>(2); + if (buildChain( + new Pair<>(rb.getString("the.input"), + (X509Certificate) certToVerify), + chain, certs)) { + for (Pair p : chain) { + checkWeak(p.fst, p.snd); + } + Certificate[] newChain = + new Certificate[chain.size()]; // buildChain() returns chain with self-signed root-cert first and // user-cert last, so we need to invert the chain before we store // it int j=0; for (int i=chain.size()-1; i>=0; i--) { - newChain[j] = chain.elementAt(i); + newChain[j] = chain.elementAt(i).snd; j++; } return newChain; @@ -3457,7 +3583,17 @@ public final class Main { } /** - * Recursively tries to establish chain from pool of trusted certs. + * Recursively tries to establish chain from pool of certs starting from + * certToVerify until a self-signed cert is found, and fill the certs found + * into chain. Each cert in the chain signs the next one. + * + * This method is able to recover from an error, say, if certToVerify + * is signed by certA but certA has no issuer in certs and itself is not + * self-signed, the method can try another certB that also signs + * certToVerify and look for signer of certB, etc, etc. + * + * Each cert in chain comes with a label showing its origin. The label is + * used in the warning message when the cert is considered a risk. * * @param certToVerify the cert that needs to be verified. * @param chain the chain that's being built. @@ -3465,19 +3601,20 @@ public final class Main { * * @return true if successful, false otherwise. */ - private boolean buildChain(X509Certificate certToVerify, - Vector chain, - Hashtable> certs) { - Principal issuer = certToVerify.getIssuerDN(); - if (KeyStoreUtil.isSelfSigned(certToVerify)) { + private boolean buildChain(Pair certToVerify, + Vector> chain, + Hashtable>> certs) { + if (KeyStoreUtil.isSelfSigned(certToVerify.snd)) { // reached self-signed root cert; // no verification needed because it's trusted. chain.addElement(certToVerify); return true; } + Principal issuer = certToVerify.snd.getIssuerDN(); + // Get the issuer's certificate(s) - Vector vec = certs.get(issuer); + Vector> vec = certs.get(issuer); if (vec == null) { return false; } @@ -3485,13 +3622,12 @@ public final class Main { // Try out each certificate in the vector, until we find one // whose public key verifies the signature of the certificate // in question. - for (Enumeration issuerCerts = vec.elements(); - issuerCerts.hasMoreElements(); ) { - X509Certificate issuerCert - = (X509Certificate)issuerCerts.nextElement(); - PublicKey issuerPubKey = issuerCert.getPublicKey(); + for (Enumeration> issuerCerts = vec.elements(); + issuerCerts.hasMoreElements(); ) { + Pair issuerCert = issuerCerts.nextElement(); + PublicKey issuerPubKey = issuerCert.snd.getPublicKey(); try { - certToVerify.verify(issuerPubKey); + certToVerify.snd.verify(issuerPubKey); } catch (Exception e) { continue; } @@ -3541,10 +3677,11 @@ public final class Main { /** * Stores the (leaf) certificates of a keystore in a hashtable. * All certs belonging to the same CA are stored in a vector that - * in turn is stored in the hashtable, keyed by the CA's subject DN + * in turn is stored in the hashtable, keyed by the CA's subject DN. + * Each cert comes with a string label that shows its origin and alias. */ private void keystorecerts2Hashtable(KeyStore ks, - Hashtable> hash) + Hashtable>> hash) throws Exception { for (Enumeration aliases = ks.aliases(); @@ -3553,13 +3690,20 @@ public final class Main { Certificate cert = ks.getCertificate(alias); if (cert != null) { Principal subjectDN = ((X509Certificate)cert).getSubjectDN(); - Vector vec = hash.get(subjectDN); + Pair pair = new Pair<>( + String.format( + rb.getString(ks == caks ? + "alias.in.cacerts" : + "alias.in.keystore"), + alias), + (X509Certificate)cert); + Vector> vec = hash.get(subjectDN); if (vec == null) { - vec = new Vector(); - vec.addElement(cert); + vec = new Vector<>(); + vec.addElement(pair); } else { - if (!vec.contains(cert)) { - vec.addElement(cert); + if (!vec.contains(pair)) { + vec.addElement(pair); } } hash.put(subjectDN, vec); @@ -4157,6 +4301,67 @@ public final class Main { return result; } + private void checkWeak(String label, String sigAlg, Key key) { + + if (!DISABLED_CHECK.permits(SIG_PRIMITIVE_SET, sigAlg, null)) { + weakWarnings.add(String.format( + rb.getString("whose.sigalg.risk"), label, sigAlg)); + } + if (key != null && !DISABLED_CHECK.permits(SIG_PRIMITIVE_SET, key)) { + weakWarnings.add(String.format( + rb.getString("whose.key.risk"), + label, + String.format(rb.getString("key.bit"), + KeyUtil.getKeySize(key), key.getAlgorithm()))); + } + } + + private void checkWeak(String label, Certificate[] certs) { + for (int i = 0; i < certs.length; i++) { + Certificate cert = certs[i]; + if (cert instanceof X509Certificate) { + X509Certificate xc = (X509Certificate)cert; + String fullLabel = label; + if (certs.length > 1) { + fullLabel = oneInMany(label, i, certs.length); + } + checkWeak(fullLabel, xc.getSigAlgName(), xc.getPublicKey()); + } + } + } + + private void checkWeak(String label, Certificate cert) { + if (cert instanceof X509Certificate) { + X509Certificate xc = (X509Certificate)cert; + checkWeak(label, xc.getSigAlgName(), xc.getPublicKey()); + } + } + + private void checkWeak(String label, PKCS10 p10) { + checkWeak(label, p10.getSigAlg(), p10.getSubjectPublicKeyInfo()); + } + + private void checkWeak(String label, CRL crl, Key key) { + if (crl instanceof X509CRLImpl) { + X509CRLImpl impl = (X509CRLImpl)crl; + checkWeak(label, impl.getSigAlgName(), key); + } + } + + private void printWeakWarnings(boolean newLine) { + if (!weakWarnings.isEmpty() && !nowarn) { + System.err.println("\nWarning:"); + for (String warning : weakWarnings) { + System.err.println(warning); + } + if (newLine) { + // When calling before a yes/no prompt, add a new line + System.err.println(); + } + } + weakWarnings.clear(); + } + /** * Prints the usage of this tool. */ diff --git a/jdk/src/java.base/share/classes/sun/security/tools/keytool/Resources.java b/jdk/src/java.base/share/classes/sun/security/tools/keytool/Resources.java index 8ede53992d8..522449c366b 100644 --- a/jdk/src/java.base/share/classes/sun/security/tools/keytool/Resources.java +++ b/jdk/src/java.base/share/classes/sun/security/tools/keytool/Resources.java @@ -360,8 +360,6 @@ public class Resources extends java.util.ListResourceBundle { {"Enter.alias.name.", "Enter alias name: "}, {".RETURN.if.same.as.for.otherAlias.", "\t(RETURN if same as for <{0}>)"}, - {".PATTERN.printX509Cert", - "Owner: {0}\nIssuer: {1}\nSerial number: {2}\nValid from: {3} until: {4}\nCertificate fingerprints:\n\t SHA1: {5}\n\t SHA256: {6}\nSignature algorithm name: {7}\nSubject Public Key Algorithm: {8} ({9,number,#})\nVersion: {10}"}, {"What.is.your.first.and.last.name.", "What is your first and last name?"}, {"What.is.the.name.of.your.organizational.unit.", @@ -428,16 +426,12 @@ public class Resources extends java.util.ListResourceBundle { {"Please.provide.keysize.for.secret.key.generation", "Please provide -keysize for secret key generation"}, - {"verified.by.s.in.s", "Verified by %s in %s"}, {"warning.not.verified.make.sure.keystore.is.correct", "WARNING: not verified. Make sure -keystore is correct."}, {"Extensions.", "Extensions: "}, {".Empty.value.", "(Empty value)"}, {"Extension.Request.", "Extension Request:"}, - {"PKCS.10.Certificate.Request.Version.1.0.Subject.s.Public.Key.s.format.s.key.", - "PKCS #10 Certificate Request (Version 1.0)\n" + - "Subject: %s\nPublic Key: %s format %s key\n"}, {"Unknown.keyUsage.type.", "Unknown keyUsage type: "}, {"Unknown.extendedkeyUsage.type.", "Unknown extendedkeyUsage type: "}, {"Unknown.AccessDescription.type.", "Unknown AccessDescription type: "}, @@ -446,7 +440,34 @@ public class Resources extends java.util.ListResourceBundle { "This extension cannot be marked as critical. "}, {"Odd.number.of.hex.digits.found.", "Odd number of hex digits found: "}, {"Unknown.extension.type.", "Unknown extension type: "}, - {"command.{0}.is.ambiguous.", "command {0} is ambiguous:"} + {"command.{0}.is.ambiguous.", "command {0} is ambiguous:"}, + + // 8171319: keytool should print out warnings when reading or + // generating cert/cert req using weak algorithms + {"the.certificate.request", "The certificate request"}, + {"the.issuer", "The issuer"}, + {"the.generated.certificate", "The generated certificate"}, + {"the.generated.crl", "The generated CRL"}, + {"the.generated.certificate.request", "The generated certificate request"}, + {"the.certificate", "The certificate"}, + {"the.crl", "The CRL"}, + {"the.tsa.certificate", "The TSA certificate"}, + {"the.input", "The input"}, + {"reply", "Reply"}, + {"one.in.many", "%s #%d of %d"}, + {"alias.in.cacerts", "Issuer <%s> in cacerts"}, + {"alias.in.keystore", "Issuer <%s>"}, + {"with.weak", "%s (weak)"}, + {"key.bit", "%d-bit %s key"}, + {"key.bit.weak", "%d-bit %s key (weak)"}, + {".PATTERN.printX509Cert.with.weak", + "Owner: {0}\nIssuer: {1}\nSerial number: {2}\nValid from: {3} until: {4}\nCertificate fingerprints:\n\t SHA1: {5}\n\t SHA256: {6}\nSignature algorithm name: {7}\nSubject Public Key Algorithm: {8}\nVersion: {9}"}, + {"PKCS.10.with.weak", + "PKCS #10 Certificate Request (Version 1.0)\n" + + "Subject: %s\nFormat: %s\nPublic Key: %s\nSignature algorithm: %s\n"}, + {"verified.by.s.in.s.weak", "Verified by %s in %s with a %s"}, + {"whose.sigalg.risk", "%s uses the %s signature algorithm which is considered a security risk."}, + {"whose.key.risk", "%s uses a %s which is considered a security risk."}, }; diff --git a/jdk/src/java.base/share/classes/sun/security/x509/X509CRLImpl.java b/jdk/src/java.base/share/classes/sun/security/x509/X509CRLImpl.java index 1fc5bed4c83..364d47b5f4d 100644 --- a/jdk/src/java.base/share/classes/sun/security/x509/X509CRLImpl.java +++ b/jdk/src/java.base/share/classes/sun/security/x509/X509CRLImpl.java @@ -536,13 +536,18 @@ public class X509CRLImpl extends X509CRL implements DerEncoder { * @return value of this CRL in a printable form. */ public String toString() { + return toStringWithAlgName("" + sigAlgId); + } + + // Specifically created for keytool to append a (weak) label to sigAlg + public String toStringWithAlgName(String name) { StringBuilder sb = new StringBuilder(); sb.append("X.509 CRL v") .append(version+1) .append('\n'); if (sigAlgId != null) sb.append("Signature Algorithm: ") - .append(sigAlgId) + .append(name) .append(", OID=") .append(sigAlgId.getOID()) .append('\n'); diff --git a/jdk/test/sun/security/tools/jarsigner/TimestampCheck.java b/jdk/test/sun/security/tools/jarsigner/TimestampCheck.java index 8cb2541f151..928bc5aa199 100644 --- a/jdk/test/sun/security/tools/jarsigner/TimestampCheck.java +++ b/jdk/test/sun/security/tools/jarsigner/TimestampCheck.java @@ -43,6 +43,7 @@ import java.util.List; import java.util.jar.JarEntry; import java.util.jar.JarFile; +import jdk.test.lib.SecurityTools; import jdk.testlibrary.*; import jdk.testlibrary.JarUtils; import sun.security.pkcs.ContentInfo; @@ -66,6 +67,7 @@ import sun.security.x509.X500Name; * java.base/sun.security.util * java.base/sun.security.tools.keytool * @library /lib/testlibrary + * @library /test/lib * @run main/timeout=600 TimestampCheck */ public class TimestampCheck { @@ -457,6 +459,18 @@ public class TimestampCheck { verify(file, "-J-Djava.security.debug=jar") .shouldHaveExitValue(0) .shouldMatch("SignatureException:.*disabled"); + + // For 8171319: keytool should print out warnings when reading or + // generating cert/cert req using weak algorithms. + // Must call keytool the command, otherwise doPrintCert() might not + // be able to reset "jdk.certpath.disabledAlgorithms". + String sout = SecurityTools.keytool("-printcert -jarfile weak.jar") + .stderrShouldContain("The TSA certificate uses a 512-bit RSA key" + + " which is considered a security risk.") + .getStdout(); + if (sout.indexOf("weak", sout.indexOf("Timestamp:")) < 0) { + throw new RuntimeException("timestamp not weak: " + sout); + } } static void checkHalfWeak(String file) throws Throwable { diff --git a/jdk/test/sun/security/tools/keytool/WeakAlg.java b/jdk/test/sun/security/tools/keytool/WeakAlg.java new file mode 100644 index 00000000000..95603ba45ea --- /dev/null +++ b/jdk/test/sun/security/tools/keytool/WeakAlg.java @@ -0,0 +1,557 @@ +/* + * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8171319 + * @summary keytool should print out warnings when reading or generating + * cert/cert req using weak algorithms + * @library /test/lib + * @modules java.base/sun.security.tools.keytool + * java.base/sun.security.tools + * java.base/sun.security.util + * @run main/othervm/timeout=600 -Duser.language=en -Duser.country=US WeakAlg + */ + +import jdk.test.lib.SecurityTools; +import jdk.test.lib.process.OutputAnalyzer; +import sun.security.tools.KeyStoreUtil; +import sun.security.util.DisabledAlgorithmConstraints; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.PrintStream; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; +import java.security.CryptoPrimitive; +import java.security.KeyStore; +import java.security.cert.X509Certificate; +import java.util.Collections; +import java.util.EnumSet; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +public class WeakAlg { + + public static void main(String[] args) throws Throwable { + + rm("ks"); + + // -genkeypair, and -printcert, -list -alias, -exportcert + // (w/ different formats) + checkGenKeyPair("a", "-keyalg RSA -sigalg MD5withRSA", "MD5withRSA"); + checkGenKeyPair("b", "-keyalg RSA -keysize 512", "512-bit RSA key"); + checkGenKeyPair("c", "-keyalg RSA", null); + + kt("-list") + .shouldContain("Warning:") + .shouldMatch(".*MD5withRSA.*risk") + .shouldMatch(".*512-bit RSA key.*risk"); + kt("-list -v") + .shouldContain("Warning:") + .shouldMatch(".*MD5withRSA.*risk") + .shouldContain("MD5withRSA (weak)") + .shouldMatch(".*512-bit RSA key.*risk") + .shouldContain("512-bit RSA key (weak)"); + + // Multiple warnings for multiple cert in -printcert or -list or -exportcert + + // -certreq, -printcertreq, -gencert + checkCertReq("a", "", null); + gencert("c-a", "") + .shouldNotContain("Warning"); // new sigalg is not weak + gencert("c-a", "-sigalg MD2withRSA") + .shouldContain("Warning:") + .shouldMatch("The generated certificate.*MD2withRSA.*risk"); + + checkCertReq("a", "-sigalg MD5withRSA", "MD5withRSA"); + gencert("c-a", "") + .shouldContain("Warning:") + .shouldMatch("The certificate request.*MD5withRSA.*risk"); + gencert("c-a", "-sigalg MD2withRSA") + .shouldContain("Warning:") + .shouldMatch("The certificate request.*MD5withRSA.*risk") + .shouldMatch("The generated certificate.*MD2withRSA.*risk"); + + checkCertReq("b", "", "512-bit RSA key"); + gencert("c-b", "") + .shouldContain("Warning:") + .shouldMatch("The certificate request.*512-bit RSA key.*risk") + .shouldMatch("The generated certificate.*512-bit RSA key.*risk"); + + checkCertReq("c", "", null); + gencert("a-c", "") + .shouldContain("Warning:") + .shouldMatch("The issuer.*MD5withRSA.*risk"); + + // but the new cert is not weak + kt("-printcert -file a-c.cert") + .shouldNotContain("Warning") + .shouldNotContain("weak"); + + gencert("b-c", "") + .shouldContain("Warning:") + .shouldMatch("The issuer.*512-bit RSA key.*risk"); + + // -importcert + checkImport(); + + // -importkeystore + checkImportKeyStore(); + + // -gencrl, -printcrl + + checkGenCRL("a", "", null); + checkGenCRL("a", "-sigalg MD5withRSA", "MD5withRSA"); + checkGenCRL("b", "", "512-bit RSA key"); + checkGenCRL("c", "", null); + + kt("-delete -alias b"); + kt("-printcrl -file b.crl") + .shouldContain("WARNING: not verified"); + } + + static void checkImportKeyStore() throws Exception { + + saveStore(); + + rm("ks"); + kt("-importkeystore -srckeystore ks2 -srcstorepass changeit") + .shouldContain("3 entries successfully imported") + .shouldContain("Warning") + .shouldMatch(".*512-bit RSA key.*risk") + .shouldMatch(".*MD5withRSA.*risk"); + + rm("ks"); + kt("-importkeystore -srckeystore ks2 -srcstorepass changeit -srcalias a") + .shouldContain("Warning") + .shouldMatch(".*MD5withRSA.*risk"); + + reStore(); + } + + static void checkImport() throws Exception { + + saveStore(); + + // add trusted cert + + // cert already in + kt("-importcert -alias d -file a.cert", "no") + .shouldContain("Certificate already exists in keystore") + .shouldContain("Warning") + .shouldMatch("The input.*MD5withRSA.*risk") + .shouldContain("Do you still want to add it?"); + kt("-importcert -alias d -file a.cert -noprompt") + .shouldContain("Warning") + .shouldMatch("The input.*MD5withRSA.*risk") + .shouldNotContain("[no]"); + + // cert is self-signed + kt("-delete -alias a"); + kt("-delete -alias d"); + kt("-importcert -alias d -file a.cert", "no") + .shouldContain("Warning") + .shouldContain("MD5withRSA (weak)") + .shouldMatch("The input.*MD5withRSA.*risk") + .shouldContain("Trust this certificate?"); + kt("-importcert -alias d -file a.cert -noprompt") + .shouldContain("Warning") + .shouldMatch("The input.*MD5withRSA.*risk") + .shouldNotContain("[no]"); + + // cert is self-signed cacerts + String weakSigAlgCA = null; + KeyStore ks = KeyStoreUtil.getCacertsKeyStore(); + if (ks != null) { + DisabledAlgorithmConstraints disabledCheck = + new DisabledAlgorithmConstraints( + DisabledAlgorithmConstraints.PROPERTY_CERTPATH_DISABLED_ALGS); + Set sigPrimitiveSet = Collections + .unmodifiableSet(EnumSet.of(CryptoPrimitive.SIGNATURE)); + + for (String s : Collections.list(ks.aliases())) { + if (ks.isCertificateEntry(s)) { + X509Certificate c = (X509Certificate)ks.getCertificate(s); + String sigAlg = c.getSigAlgName(); + if (!disabledCheck.permits(sigPrimitiveSet, sigAlg, null)) { + weakSigAlgCA = sigAlg; + Files.write(Paths.get("ca.cert"), + ks.getCertificate(s).getEncoded()); + break; + } + } + } + } + if (weakSigAlgCA != null) { + kt("-delete -alias d"); + kt("-importcert -alias d -trustcacerts -file ca.cert", "no") + .shouldContain("Certificate already exists in system-wide CA") + .shouldContain("Warning") + .shouldMatch("The input.*" + weakSigAlgCA + ".*risk") + .shouldContain("Do you still want to add it to your own keystore?"); + kt("-importcert -alias d -file ca.cert -noprompt") + .shouldContain("Warning") + .shouldMatch("The input.*" + weakSigAlgCA + ".*risk") + .shouldNotContain("[no]"); + } + + // a non self-signed weak cert + reStore(); + certreq("b", ""); + gencert("c-b", ""); + kt("-importcert -alias d -file c-b.cert") // weak only, no prompt + .shouldContain("Warning") + .shouldNotContain("512-bit RSA key (weak)") + .shouldMatch("The input.*512-bit RSA key.*risk") + .shouldNotContain("[no]"); + + kt("-delete -alias b"); + kt("-delete -alias c"); + kt("-delete -alias d"); + + kt("-importcert -alias d -file c-b.cert", "no") // weak and not trusted + .shouldContain("Warning") + .shouldContain("512-bit RSA key (weak)") + .shouldMatch("The input.*512-bit RSA key.*risk") + .shouldContain("Trust this certificate?"); + kt("-importcert -alias d -file c-b.cert -noprompt") + .shouldContain("Warning") + .shouldMatch("The input.*512-bit RSA key.*risk") + .shouldNotContain("[no]"); + + // a non self-signed strong cert + reStore(); + certreq("a", ""); + gencert("c-a", ""); + kt("-importcert -alias d -file c-a.cert") // trusted + .shouldNotContain("Warning") + .shouldNotContain("[no]"); + + kt("-delete -alias a"); + kt("-delete -alias c"); + kt("-delete -alias d"); + + kt("-importcert -alias d -file c-a.cert", "no") // not trusted + .shouldNotContain("Warning") + .shouldContain("Trust this certificate?"); + kt("-importcert -alias d -file c-a.cert -noprompt") + .shouldNotContain("Warning") + .shouldNotContain("[no]"); + + // install reply + + reStore(); + + gencert("a-b", ""); + gencert("b-c", ""); + + // Full chain with root + cat("a-a-b-c.cert", "b-c.cert", "a-b.cert", "a.cert"); + kt("-importcert -alias c -file a-a-b-c.cert") // only weak + .shouldContain("Warning") + .shouldMatch("Reply #2 of 3.*512-bit RSA key.*risk") + .shouldMatch("Reply #3 of 3.*MD5withRSA.*risk") + .shouldNotContain("[no]"); + + // Without root + cat("a-b-c.cert", "b-c.cert", "a-b.cert"); + kt("-importcert -alias c -file a-b-c.cert") // only weak + .shouldContain("Warning") + .shouldMatch("Reply #2 of 2.*512-bit RSA key.*risk") + .shouldMatch("Issuer .*MD5withRSA.*risk") + .shouldNotContain("[no]"); + + reStore(); + gencert("b-a", ""); + + kt("-importcert -alias a -file b-a.cert") + .shouldContain("Warning") + .shouldMatch("Issuer .*512-bit RSA key.*risk") + .shouldNotContain("[no]"); + + kt("-importcert -alias a -file c-a.cert") + .shouldNotContain("Warning"); + + kt("-importcert -alias b -file c-b.cert") + .shouldContain("Warning") + .shouldMatch("The input.*512-bit RSA key.*risk") + .shouldNotContain("[no]"); + + reStore(); + gencert("b-a", ""); + + cat("c-b-a.cert", "b-a.cert", "c-b.cert"); + + kt("-printcert -file c-b-a.cert") + .shouldContain("Warning") + .shouldMatch("The certificate #2 of 2.*512-bit RSA key.*risk"); + + kt("-delete -alias b"); + + kt("-importcert -alias a -file c-b-a.cert") + .shouldContain("Warning") + .shouldMatch("Reply #2 of 2.*512-bit RSA key.*risk") + .shouldNotContain("[no]"); + + kt("-delete -alias c"); + kt("-importcert -alias a -file c-b-a.cert", "no") + .shouldContain("Top-level certificate in reply:") + .shouldContain("512-bit RSA key (weak)") + .shouldContain("Warning") + .shouldMatch("Reply #2 of 2.*512-bit RSA key.*risk") + .shouldContain("Install reply anyway?"); + kt("-importcert -alias a -file c-b-a.cert -noprompt") + .shouldContain("Warning") + .shouldMatch("Reply #2 of 2.*512-bit RSA key.*risk") + .shouldNotContain("[no]"); + + reStore(); + } + + private static void cat(String dest, String... src) throws IOException { + System.out.println("---------------------------------------------"); + System.out.printf("$ cat "); + + ByteArrayOutputStream bout = new ByteArrayOutputStream(); + for (String s : src) { + System.out.printf(s + " "); + bout.write(Files.readAllBytes(Paths.get(s))); + } + Files.write(Paths.get(dest), bout.toByteArray()); + System.out.println("> " + dest); + } + + static void checkGenCRL(String alias, String options, String bad) { + + OutputAnalyzer oa = kt("-gencrl -alias " + alias + + " -id 1 -file " + alias + ".crl " + options); + if (bad == null) { + oa.shouldNotContain("Warning"); + } else { + oa.shouldContain("Warning") + .shouldMatch("The generated CRL.*" + bad + ".*risk"); + } + + oa = kt("-printcrl -file " + alias + ".crl"); + if (bad == null) { + oa.shouldNotContain("Warning") + .shouldContain("Verified by " + alias + " in keystore") + .shouldNotContain("(weak"); + } else { + oa.shouldContain("Warning:") + .shouldMatch("The CRL.*" + bad + ".*risk") + .shouldContain("Verified by " + alias + " in keystore") + .shouldContain(bad + " (weak)"); + } + } + + static void checkCertReq( + String alias, String options, String bad) { + + OutputAnalyzer oa = certreq(alias, options); + if (bad == null) { + oa.shouldNotContain("Warning"); + } else { + oa.shouldContain("Warning") + .shouldMatch("The generated certificate request.*" + bad + ".*risk"); + } + + oa = kt("-printcertreq -file " + alias + ".req"); + if (bad == null) { + oa.shouldNotContain("Warning") + .shouldNotContain("(weak)"); + } else { + oa.shouldContain("Warning") + .shouldMatch("The certificate request.*" + bad + ".*risk") + .shouldContain(bad + " (weak)"); + } + } + + static void checkGenKeyPair( + String alias, String options, String bad) { + + OutputAnalyzer oa = genkeypair(alias, options); + if (bad == null) { + oa.shouldNotContain("Warning"); + } else { + oa.shouldContain("Warning") + .shouldMatch("The generated certificate.*" + bad + ".*risk"); + } + + oa = kt("-exportcert -alias " + alias + " -file " + alias + ".cert"); + if (bad == null) { + oa.shouldNotContain("Warning"); + } else { + oa.shouldContain("Warning") + .shouldMatch("The certificate.*" + bad + ".*risk"); + } + + oa = kt("-exportcert -rfc -alias " + alias + " -file " + alias + ".cert"); + if (bad == null) { + oa.shouldNotContain("Warning"); + } else { + oa.shouldContain("Warning") + .shouldMatch("The certificate.*" + bad + ".*risk"); + } + + oa = kt("-printcert -rfc -file " + alias + ".cert"); + if (bad == null) { + oa.shouldNotContain("Warning"); + } else { + oa.shouldContain("Warning") + .shouldMatch("The certificate.*" + bad + ".*risk"); + } + + oa = kt("-list -alias " + alias); + if (bad == null) { + oa.shouldNotContain("Warning"); + } else { + oa.shouldContain("Warning") + .shouldMatch("The certificate.*" + bad + ".*risk"); + } + + // With cert content + + oa = kt("-printcert -file " + alias + ".cert"); + if (bad == null) { + oa.shouldNotContain("Warning"); + } else { + oa.shouldContain("Warning") + .shouldContain(bad + " (weak)") + .shouldMatch("The certificate.*" + bad + ".*risk"); + } + + oa = kt("-list -v -alias " + alias); + if (bad == null) { + oa.shouldNotContain("Warning"); + } else { + oa.shouldContain("Warning") + .shouldContain(bad + " (weak)") + .shouldMatch("The certificate.*" + bad + ".*risk"); + } + } + + // This is slow, but real keytool process is launched. + static OutputAnalyzer kt1(String cmd, String... input) { + cmd = "-keystore ks -storepass changeit " + + "-keypass changeit " + cmd; + System.out.println("---------------------------------------------"); + try { + SecurityTools.setResponse(input); + return SecurityTools.keytool(cmd); + } catch (Throwable e) { + throw new RuntimeException(e); + } + } + + // Fast keytool execution by directly calling its main() method + static OutputAnalyzer kt(String cmd, String... input) { + PrintStream out = System.out; + PrintStream err = System.err; + InputStream ins = System.in; + ByteArrayOutputStream bout = new ByteArrayOutputStream(); + ByteArrayOutputStream berr = new ByteArrayOutputStream(); + boolean succeed = true; + try { + cmd = "-keystore ks -storepass changeit " + + "-keypass changeit " + cmd; + System.out.println("---------------------------------------------"); + System.out.println("$ keytool " + cmd); + System.out.println(); + String feed = ""; + if (input.length > 0) { + feed = Stream.of(input).collect(Collectors.joining("\n")) + "\n"; + } + System.setIn(new ByteArrayInputStream(feed.getBytes())); + System.setOut(new PrintStream(bout)); + System.setErr(new PrintStream(berr)); + sun.security.tools.keytool.Main.main( + cmd.trim().split("\\s+")); + } catch (Exception e) { + // Might be a normal exception when -debug is on or + // SecurityException (thrown by jtreg) when System.exit() is called + if (!(e instanceof SecurityException)) { + e.printStackTrace(); + } + succeed = false; + } finally { + System.setOut(out); + System.setErr(err); + System.setIn(ins); + } + String sout = new String(bout.toByteArray()); + String serr = new String(berr.toByteArray()); + System.out.println("STDOUT:\n" + sout + "\nSTDERR:\n" + serr); + if (!succeed) { + throw new RuntimeException(); + } + return new OutputAnalyzer(sout, serr); + } + + static OutputAnalyzer genkeypair(String alias, String options) { + return kt("-genkeypair -alias " + alias + " -dname CN=" + alias + + " -keyalg RSA -storetype JKS " + options); + } + + static OutputAnalyzer certreq(String alias, String options) { + return kt("-certreq -alias " + alias + + " -file " + alias + ".req " + options); + } + + static OutputAnalyzer exportcert(String alias) { + return kt("-exportcert -alias " + alias + " -file " + alias + ".cert"); + } + + static OutputAnalyzer gencert(String relation, String options) { + int pos = relation.indexOf("-"); + String issuer = relation.substring(0, pos); + String subject = relation.substring(pos + 1); + return kt(" -gencert -alias " + issuer + " -infile " + subject + + ".req -outfile " + relation + ".cert " + options); + } + + static void saveStore() throws IOException { + System.out.println("---------------------------------------------"); + System.out.println("$ cp ks ks2"); + Files.copy(Paths.get("ks"), Paths.get("ks2"), + StandardCopyOption.REPLACE_EXISTING); + } + + static void reStore() throws IOException { + System.out.println("---------------------------------------------"); + System.out.println("$ cp ks2 ks"); + Files.copy(Paths.get("ks2"), Paths.get("ks"), + StandardCopyOption.REPLACE_EXISTING); + } + + static void rm(String s) throws IOException { + System.out.println("---------------------------------------------"); + System.out.println("$ rm " + s); + Files.deleteIfExists(Paths.get(s)); + } +} From d64c22ae617b48dce438cb282861efd626e81abd Mon Sep 17 00:00:00 2001 From: Weijun Wang Date: Tue, 7 Mar 2017 22:55:36 +0800 Subject: [PATCH 11/11] 8175846: Provide javadoc descriptions for jdk.policytool and jdk.crypto.* modules Reviewed-by: vinnie --- .../jdk.crypto.cryptoki/share/classes/module-info.java | 5 +++++ jdk/src/jdk.crypto.ec/share/classes/module-info.java | 7 ++++++- .../jdk.crypto.mscapi/windows/classes/module-info.java | 7 ++++++- .../solaris/classes/module-info.java | 7 ++++++- jdk/src/jdk.policytool/share/classes/module-info.java | 10 +++++++++- 5 files changed, 32 insertions(+), 4 deletions(-) diff --git a/jdk/src/jdk.crypto.cryptoki/share/classes/module-info.java b/jdk/src/jdk.crypto.cryptoki/share/classes/module-info.java index 845ae002c77..487df00c299 100644 --- a/jdk/src/jdk.crypto.cryptoki/share/classes/module-info.java +++ b/jdk/src/jdk.crypto.cryptoki/share/classes/module-info.java @@ -23,6 +23,11 @@ * questions. */ +/** + * The SunPKCS11 security provider. + * + * @since 9 + */ module jdk.crypto.cryptoki { // Depends on SunEC provider for EC related functionality requires jdk.crypto.ec; diff --git a/jdk/src/jdk.crypto.ec/share/classes/module-info.java b/jdk/src/jdk.crypto.ec/share/classes/module-info.java index 227882d5dbc..74601fa8faf 100644 --- a/jdk/src/jdk.crypto.ec/share/classes/module-info.java +++ b/jdk/src/jdk.crypto.ec/share/classes/module-info.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,6 +23,11 @@ * questions. */ +/** + * The SunEC security provider. + * + * @since 9 + */ module jdk.crypto.ec { provides java.security.Provider with sun.security.ec.SunEC; } diff --git a/jdk/src/jdk.crypto.mscapi/windows/classes/module-info.java b/jdk/src/jdk.crypto.mscapi/windows/classes/module-info.java index 9a2801e8096..20222f45122 100644 --- a/jdk/src/jdk.crypto.mscapi/windows/classes/module-info.java +++ b/jdk/src/jdk.crypto.mscapi/windows/classes/module-info.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,6 +23,11 @@ * questions. */ +/** + * The SunMSCAPI security provider. + * + * @since 9 + */ module jdk.crypto.mscapi { provides java.security.Provider with sun.security.mscapi.SunMSCAPI; } diff --git a/jdk/src/jdk.crypto.ucrypto/solaris/classes/module-info.java b/jdk/src/jdk.crypto.ucrypto/solaris/classes/module-info.java index 161c6fe56ed..21d8a6686a2 100644 --- a/jdk/src/jdk.crypto.ucrypto/solaris/classes/module-info.java +++ b/jdk/src/jdk.crypto.ucrypto/solaris/classes/module-info.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,6 +23,11 @@ * questions. */ +/** + * The OracleUCrypto security provider. + * + * @since 9 + */ module jdk.crypto.ucrypto { provides java.security.Provider with com.oracle.security.ucrypto.UcryptoProvider; } diff --git a/jdk/src/jdk.policytool/share/classes/module-info.java b/jdk/src/jdk.policytool/share/classes/module-info.java index f972e1cd15a..8ae0f88aa3a 100644 --- a/jdk/src/jdk.policytool/share/classes/module-info.java +++ b/jdk/src/jdk.policytool/share/classes/module-info.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,6 +23,14 @@ * questions. */ +/** + * GUI tool for managing policy files. + * + * @since 9 + * @deprecated The policytool tool has been deprecated and + * is planned to be removed in a future release. + */ +@Deprecated module jdk.policytool { requires java.desktop; requires java.logging;