From 10f59695cf4d1e578450564d6bf84589b7ac4e93 Mon Sep 17 00:00:00 2001 From: Viktor Klang Date: Mon, 13 Jan 2025 17:27:20 +0100 Subject: [PATCH 1/2] Addresses IAE and NPE messages for ThreadPoolExecutor --- .../util/concurrent/ThreadPoolExecutor.java | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/java.base/share/classes/java/util/concurrent/ThreadPoolExecutor.java b/src/java.base/share/classes/java/util/concurrent/ThreadPoolExecutor.java index e34810fe96659..e1f223dc74197 100644 --- a/src/java.base/share/classes/java/util/concurrent/ThreadPoolExecutor.java +++ b/src/java.base/share/classes/java/util/concurrent/ThreadPoolExecutor.java @@ -1256,13 +1256,17 @@ public ThreadPoolExecutor(int corePoolSize, BlockingQueue workQueue, ThreadFactory threadFactory, RejectedExecutionHandler handler) { - if (corePoolSize < 0 || - maximumPoolSize <= 0 || - maximumPoolSize < corePoolSize || - keepAliveTime < 0) - throw new IllegalArgumentException(); - if (workQueue == null || threadFactory == null || handler == null) - throw new NullPointerException(); + if (corePoolSize < 0) + throw new IllegalArgumentException("corePoolSize must be non-negative"); + if (maximumPoolSize <= 0 || maximumPoolSize < corePoolSize) + throw new IllegalArgumentException("maximumPoolSize must be positive and greater-or-equal to corePoolSize"); + if (keepAliveTime < 0) + throw new IllegalArgumentException("keepAliveTime must not be negative"); + + Objects.requireNonNull(workQueue, "workQueue"); + Objects.requireNonNull(threadFactory, "threadFactory"); + Objects.requireNonNull(handler, "handler"); + this.corePoolSize = corePoolSize; this.maximumPoolSize = maximumPoolSize; this.workQueue = workQueue; @@ -1289,8 +1293,7 @@ public ThreadPoolExecutor(int corePoolSize, * @throws NullPointerException if {@code command} is null */ public void execute(Runnable command) { - if (command == null) - throw new NullPointerException(); + Objects.requireNonNull(command, "command"); /* * Proceed in 3 steps: * @@ -1451,9 +1454,7 @@ protected void finalize() {} * @see #getThreadFactory */ public void setThreadFactory(ThreadFactory threadFactory) { - if (threadFactory == null) - throw new NullPointerException(); - this.threadFactory = threadFactory; + this.threadFactory = Objects.requireNonNull(threadFactory, "threadFactory"); } /** @@ -1474,9 +1475,7 @@ public ThreadFactory getThreadFactory() { * @see #getRejectedExecutionHandler */ public void setRejectedExecutionHandler(RejectedExecutionHandler handler) { - if (handler == null) - throw new NullPointerException(); - this.handler = handler; + this.handler = Objects.requireNonNull(handler, "handler"); } /** @@ -1503,8 +1502,10 @@ public RejectedExecutionHandler getRejectedExecutionHandler() { * @see #getCorePoolSize */ public void setCorePoolSize(int corePoolSize) { - if (corePoolSize < 0 || maximumPoolSize < corePoolSize) - throw new IllegalArgumentException(); + if (corePoolSize < 0) + throw new IllegalArgumentException("corePoolSize must be non-negative"); + if (maximumPoolSize < corePoolSize) + throw new IllegalArgumentException("corePoolSize must be lesser-or-equal to maximumPoolSize"); int delta = corePoolSize - this.corePoolSize; this.corePoolSize = corePoolSize; if (workerCountOf(ctl.get()) > corePoolSize) @@ -1629,7 +1630,7 @@ public void allowCoreThreadTimeOut(boolean value) { */ public void setMaximumPoolSize(int maximumPoolSize) { if (maximumPoolSize <= 0 || maximumPoolSize < corePoolSize) - throw new IllegalArgumentException(); + throw new IllegalArgumentException("maximumPoolSize must be positive and greater-or-equal to corePoolSize"); this.maximumPoolSize = maximumPoolSize; if (workerCountOf(ctl.get()) > maximumPoolSize) interruptIdleWorkers(); @@ -1663,7 +1664,7 @@ public int getMaximumPoolSize() { */ public void setKeepAliveTime(long time, TimeUnit unit) { if (time < 0) - throw new IllegalArgumentException(); + throw new IllegalArgumentException("time must be non-negative"); if (time == 0 && allowsCoreThreadTimeOut()) throw new IllegalArgumentException("Core threads must have nonzero keep alive times"); long keepAliveTime = unit.toNanos(time); From 7cf05b5b911783076ad823d8dade8cfa0fdba9c7 Mon Sep 17 00:00:00 2001 From: Viktor Klang Date: Tue, 14 Jan 2025 11:51:05 +0100 Subject: [PATCH 2/2] Updates the ThreadPoolExecutorTest.java to assert the exception messages added --- .../util/concurrent/ThreadPoolExecutor.java | 2 +- .../tck/ThreadPoolExecutorTest.java | 184 ++++++++++++++---- 2 files changed, 151 insertions(+), 35 deletions(-) diff --git a/src/java.base/share/classes/java/util/concurrent/ThreadPoolExecutor.java b/src/java.base/share/classes/java/util/concurrent/ThreadPoolExecutor.java index e1f223dc74197..2fe8ebf75f143 100644 --- a/src/java.base/share/classes/java/util/concurrent/ThreadPoolExecutor.java +++ b/src/java.base/share/classes/java/util/concurrent/ThreadPoolExecutor.java @@ -1261,7 +1261,7 @@ public ThreadPoolExecutor(int corePoolSize, if (maximumPoolSize <= 0 || maximumPoolSize < corePoolSize) throw new IllegalArgumentException("maximumPoolSize must be positive and greater-or-equal to corePoolSize"); if (keepAliveTime < 0) - throw new IllegalArgumentException("keepAliveTime must not be negative"); + throw new IllegalArgumentException("keepAliveTime must be non-negative"); Objects.requireNonNull(workQueue, "workQueue"); Objects.requireNonNull(threadFactory, "threadFactory"); diff --git a/test/jdk/java/util/concurrent/tck/ThreadPoolExecutorTest.java b/test/jdk/java/util/concurrent/tck/ThreadPoolExecutorTest.java index 73e6deda9870d..ebbc8227c6620 100644 --- a/test/jdk/java/util/concurrent/tck/ThreadPoolExecutorTest.java +++ b/test/jdk/java/util/concurrent/tck/ThreadPoolExecutorTest.java @@ -737,7 +737,9 @@ public void testConstructor1() { new ThreadPoolExecutor(-1, 1, 1L, SECONDS, new ArrayBlockingQueue(10)); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals("corePoolSize must be non-negative", success.getMessage()); + } } /** @@ -748,7 +750,12 @@ public void testConstructor2() { new ThreadPoolExecutor(1, -1, 1L, SECONDS, new ArrayBlockingQueue(10)); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals( + "maximumPoolSize must be positive and greater-or-equal to corePoolSize", + success.getMessage() + ); + } } /** @@ -759,7 +766,12 @@ public void testConstructor3() { new ThreadPoolExecutor(1, 0, 1L, SECONDS, new ArrayBlockingQueue(10)); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals( + "maximumPoolSize must be positive and greater-or-equal to corePoolSize", + success.getMessage() + ); + } } /** @@ -770,7 +782,9 @@ public void testConstructor4() { new ThreadPoolExecutor(1, 2, -1L, SECONDS, new ArrayBlockingQueue(10)); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals("keepAliveTime must be non-negative", success.getMessage()); + } } /** @@ -781,7 +795,12 @@ public void testConstructor5() { new ThreadPoolExecutor(2, 1, 1L, SECONDS, new ArrayBlockingQueue(10)); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals( + "maximumPoolSize must be positive and greater-or-equal to corePoolSize", + success.getMessage() + ); + } } /** @@ -792,7 +811,9 @@ public void testConstructorNullPointerException() { new ThreadPoolExecutor(1, 2, 1L, SECONDS, (BlockingQueue) null); shouldThrow(); - } catch (NullPointerException success) {} + } catch (NullPointerException success) { + assertEquals("workQueue", success.getMessage()); + } } /** @@ -804,7 +825,9 @@ public void testConstructor6() { new ArrayBlockingQueue(10), new SimpleThreadFactory()); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals("corePoolSize must be non-negative", success.getMessage()); + } } /** @@ -816,7 +839,12 @@ public void testConstructor7() { new ArrayBlockingQueue(10), new SimpleThreadFactory()); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals( + "maximumPoolSize must be positive and greater-or-equal to corePoolSize", + success.getMessage() + ); + } } /** @@ -828,7 +856,12 @@ public void testConstructor8() { new ArrayBlockingQueue(10), new SimpleThreadFactory()); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals( + "maximumPoolSize must be positive and greater-or-equal to corePoolSize", + success.getMessage() + ); + } } /** @@ -840,7 +873,9 @@ public void testConstructor9() { new ArrayBlockingQueue(10), new SimpleThreadFactory()); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals("keepAliveTime must be non-negative", success.getMessage()); + } } /** @@ -852,7 +887,12 @@ public void testConstructor10() { new ArrayBlockingQueue(10), new SimpleThreadFactory()); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals( + "maximumPoolSize must be positive and greater-or-equal to corePoolSize", + success.getMessage() + ); + } } /** @@ -864,7 +904,9 @@ public void testConstructorNullPointerException2() { (BlockingQueue) null, new SimpleThreadFactory()); shouldThrow(); - } catch (NullPointerException success) {} + } catch (NullPointerException success) { + assertEquals("workQueue", success.getMessage()); + } } /** @@ -876,7 +918,9 @@ public void testConstructorNullPointerException3() { new ArrayBlockingQueue(10), (ThreadFactory) null); shouldThrow(); - } catch (NullPointerException success) {} + } catch (NullPointerException success) { + assertEquals("threadFactory", success.getMessage()); + } } /** @@ -888,7 +932,9 @@ public void testConstructor11() { new ArrayBlockingQueue(10), new NoOpREHandler()); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals("corePoolSize must be non-negative", success.getMessage()); + } } /** @@ -900,7 +946,12 @@ public void testConstructor12() { new ArrayBlockingQueue(10), new NoOpREHandler()); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals( + "maximumPoolSize must be positive and greater-or-equal to corePoolSize", + success.getMessage() + ); + } } /** @@ -912,7 +963,12 @@ public void testConstructor13() { new ArrayBlockingQueue(10), new NoOpREHandler()); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals( + "maximumPoolSize must be positive and greater-or-equal to corePoolSize", + success.getMessage() + ); + } } /** @@ -924,7 +980,9 @@ public void testConstructor14() { new ArrayBlockingQueue(10), new NoOpREHandler()); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals("keepAliveTime must be non-negative", success.getMessage()); + } } /** @@ -936,7 +994,12 @@ public void testConstructor15() { new ArrayBlockingQueue(10), new NoOpREHandler()); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals( + "maximumPoolSize must be positive and greater-or-equal to corePoolSize", + success.getMessage() + ); + } } /** @@ -948,7 +1011,9 @@ public void testConstructorNullPointerException4() { (BlockingQueue) null, new NoOpREHandler()); shouldThrow(); - } catch (NullPointerException success) {} + } catch (NullPointerException success) { + assertEquals("workQueue", success.getMessage()); + } } /** @@ -960,7 +1025,9 @@ public void testConstructorNullPointerException5() { new ArrayBlockingQueue(10), (RejectedExecutionHandler) null); shouldThrow(); - } catch (NullPointerException success) {} + } catch (NullPointerException success) { + assertEquals("handler", success.getMessage()); + } } /** @@ -973,7 +1040,9 @@ public void testConstructor16() { new SimpleThreadFactory(), new NoOpREHandler()); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals("corePoolSize must be non-negative", success.getMessage()); + } } /** @@ -986,7 +1055,12 @@ public void testConstructor17() { new SimpleThreadFactory(), new NoOpREHandler()); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals( + "maximumPoolSize must be positive and greater-or-equal to corePoolSize", + success.getMessage() + ); + } } /** @@ -999,7 +1073,12 @@ public void testConstructor18() { new SimpleThreadFactory(), new NoOpREHandler()); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals( + "maximumPoolSize must be positive and greater-or-equal to corePoolSize", + success.getMessage() + ); + } } /** @@ -1012,7 +1091,9 @@ public void testConstructor19() { new SimpleThreadFactory(), new NoOpREHandler()); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals("keepAliveTime must be non-negative", success.getMessage()); + } } /** @@ -1025,7 +1106,12 @@ public void testConstructor20() { new SimpleThreadFactory(), new NoOpREHandler()); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals( + "maximumPoolSize must be positive and greater-or-equal to corePoolSize", + success.getMessage() + ); + } } /** @@ -1038,7 +1124,9 @@ public void testConstructorNullPointerException6() { new SimpleThreadFactory(), new NoOpREHandler()); shouldThrow(); - } catch (NullPointerException success) {} + } catch (NullPointerException success) { + assertEquals("workQueue", success.getMessage()); + } } /** @@ -1051,7 +1139,9 @@ public void testConstructorNullPointerException7() { new SimpleThreadFactory(), (RejectedExecutionHandler) null); shouldThrow(); - } catch (NullPointerException success) {} + } catch (NullPointerException success) { + assertEquals("handler", success.getMessage()); + } } /** @@ -1064,7 +1154,9 @@ public void testConstructorNullPointerException8() { (ThreadFactory) null, new NoOpREHandler()); shouldThrow(); - } catch (NullPointerException success) {} + } catch (NullPointerException success) { + assertEquals("threadFactory", success.getMessage()); + } } /** @@ -1228,7 +1320,9 @@ public void testCorePoolSizeIllegalArgumentException() { try { p.setCorePoolSize(-1); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals("corePoolSize must be non-negative", success.getMessage()); + } } } @@ -1245,7 +1339,12 @@ public void testMaximumPoolSizeIllegalArgumentException() { try { p.setMaximumPoolSize(1); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals( + "maximumPoolSize must be positive and greater-or-equal to corePoolSize", + success.getMessage() + ); + } } } @@ -1262,7 +1361,12 @@ public void testMaximumPoolSizeIllegalArgumentException2() { try { p.setMaximumPoolSize(-1); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals( + "maximumPoolSize must be positive and greater-or-equal to corePoolSize", + success.getMessage() + ); + } } } @@ -1282,13 +1386,23 @@ public void testPoolSizeInvariants() { try { p.setMaximumPoolSize(s - 1); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals( + "maximumPoolSize must be positive and greater-or-equal to corePoolSize", + success.getMessage() + ); + } assertEquals(s, p.getCorePoolSize()); assertEquals(s, p.getMaximumPoolSize()); try { p.setCorePoolSize(s + 1); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals( + "corePoolSize must be lesser-or-equal to maximumPoolSize", + success.getMessage() + ); + } assertEquals(s, p.getCorePoolSize()); assertEquals(s, p.getMaximumPoolSize()); } @@ -1308,7 +1422,9 @@ public void testKeepAliveTimeIllegalArgumentException() { try { p.setKeepAliveTime(-1, MILLISECONDS); shouldThrow(); - } catch (IllegalArgumentException success) {} + } catch (IllegalArgumentException success) { + assertEquals("time must be non-negative", success.getMessage()); + } } }