From 60e726bdf2cde5ca182e1efbc5da4d414bed58de Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Mon, 23 Jun 2025 11:59:55 +0200 Subject: [PATCH 1/3] Java: Add `java/javautilconcurrentscheduledthreadpoolexecutor` query for zero thread pool size --- .../java-code-quality-extended.qls.expected | 1 + .../java-code-quality.qls.expected | 1 + .../ScheduledThreadPoolExecutorZeroThread.md | 24 +++++++++++++ .../ScheduledThreadPoolExecutorZeroThread.ql | 35 +++++++++++++++++++ ...duledThreadPoolExecutorZeroThread.expected | 3 ++ ...cheduledThreadPoolExecutorZeroThread.qlref | 2 ++ .../Test.java | 11 ++++++ 7 files changed, 77 insertions(+) create mode 100644 java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.md create mode 100644 java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.ql create mode 100644 java/ql/test/query-tests/ScheduledThreadPoolExecutorZeroThread/ScheduledThreadPoolExecutorZeroThread.expected create mode 100644 java/ql/test/query-tests/ScheduledThreadPoolExecutorZeroThread/ScheduledThreadPoolExecutorZeroThread.qlref create mode 100644 java/ql/test/query-tests/ScheduledThreadPoolExecutorZeroThread/Test.java diff --git a/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected b/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected index dd15d7f3bdd4..cb7de0844dca 100644 --- a/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected +++ b/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected @@ -33,6 +33,7 @@ ql/java/ql/src/Likely Bugs/Concurrency/CallsToRunnableRun.ql ql/java/ql/src/Likely Bugs/Concurrency/DoubleCheckedLocking.ql ql/java/ql/src/Likely Bugs/Concurrency/DoubleCheckedLockingWithInitRace.ql ql/java/ql/src/Likely Bugs/Concurrency/NonSynchronizedOverride.ql +ql/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.ql ql/java/ql/src/Likely Bugs/Concurrency/SynchOnBoxedType.ql ql/java/ql/src/Likely Bugs/Concurrency/SynchSetUnsynchGet.ql ql/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql diff --git a/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected b/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected index 589041ac7b35..815575bac35c 100644 --- a/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected +++ b/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected @@ -31,6 +31,7 @@ ql/java/ql/src/Likely Bugs/Concurrency/CallsToRunnableRun.ql ql/java/ql/src/Likely Bugs/Concurrency/DoubleCheckedLocking.ql ql/java/ql/src/Likely Bugs/Concurrency/DoubleCheckedLockingWithInitRace.ql ql/java/ql/src/Likely Bugs/Concurrency/NonSynchronizedOverride.ql +ql/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.ql ql/java/ql/src/Likely Bugs/Concurrency/SynchOnBoxedType.ql ql/java/ql/src/Likely Bugs/Concurrency/SynchSetUnsynchGet.ql ql/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql diff --git a/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.md b/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.md new file mode 100644 index 000000000000..428414b8f1aa --- /dev/null +++ b/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.md @@ -0,0 +1,24 @@ +## Overview + +According the Java documentation on `ScheduledThreadPoolExecutor`, it is not a good idea to set `corePoolSize` to zero, since doing so indicates the executor to keep 0 threads in its pool and the executor will serve no purpose. + +## Recommendation + +Set the `ScheduledThreadPoolExecutor` to have 1 or more threads in its thread pool and use the class's other methods to create a thread execution schedule. + +## Example + +```java +public class Test { + void f() { + int i = 0; + ScheduledThreadPoolExecutor s = new ScheduledThreadPoolExecutor(1); // COMPLIANT + ScheduledThreadPoolExecutor s1 = new ScheduledThreadPoolExecutor(0); // NON_COMPLIANT + s.setCorePoolSize(0); // NON_COMPLIANT + s.setCorePoolSize(i); // NON_COMPLIANT + } +} +``` + +## References +- [ScheduledThreadPoolExecutor](https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/util/concurrent/ScheduledThreadPoolExecutor.html) diff --git a/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.ql b/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.ql new file mode 100644 index 000000000000..cb6928a5b809 --- /dev/null +++ b/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.ql @@ -0,0 +1,35 @@ +/** + * @id java/javautilconcurrentscheduledthreadpoolexecutor + * @name Zero threads set for `java.util.concurrent.ScheduledThreadPoolExecutor` + * @description Setting `java.util.concurrent.ScheduledThreadPoolExecutor` to have 0 threads serves + * no purpose and may indicate programmer error. + * @kind problem + * @precision very-high + * @problem.severity recommendation + * @tags quality + * reliability + * correctness + * concurrency + */ + +import java +import semmle.code.java.dataflow.DataFlow + +/** + * A `Call` that has the ability to set or modify the `corePoolSize` of the `java.util.concurrent.ScheduledThreadPoolExecutor` type. + */ +class Sink extends Call { + Sink() { + this.getCallee() + .hasQualifiedName("java.util.concurrent", "ThreadPoolExecutor", "setCorePoolSize") or + this.getCallee() + .hasQualifiedName("java.util.concurrent", "ScheduledThreadPoolExecutor", + "ScheduledThreadPoolExecutor") + } +} + +from IntegerLiteral zero, Sink set +where + DataFlow::localFlow(DataFlow::exprNode(zero), DataFlow::exprNode(set.getArgument(0))) and + zero.getIntValue() = 0 +select set, "ScheduledThreadPoolExecutor.corePoolSize is set to have 0 threads." diff --git a/java/ql/test/query-tests/ScheduledThreadPoolExecutorZeroThread/ScheduledThreadPoolExecutorZeroThread.expected b/java/ql/test/query-tests/ScheduledThreadPoolExecutorZeroThread/ScheduledThreadPoolExecutorZeroThread.expected new file mode 100644 index 000000000000..038f2d1d9988 --- /dev/null +++ b/java/ql/test/query-tests/ScheduledThreadPoolExecutorZeroThread/ScheduledThreadPoolExecutorZeroThread.expected @@ -0,0 +1,3 @@ +| Test.java:7:42:7:75 | new ScheduledThreadPoolExecutor(...) | ScheduledThreadPoolExecutor.corePoolSize is set to have 0 threads. | +| Test.java:8:9:8:28 | setCorePoolSize(...) | ScheduledThreadPoolExecutor.corePoolSize is set to have 0 threads. | +| Test.java:9:9:9:28 | setCorePoolSize(...) | ScheduledThreadPoolExecutor.corePoolSize is set to have 0 threads. | diff --git a/java/ql/test/query-tests/ScheduledThreadPoolExecutorZeroThread/ScheduledThreadPoolExecutorZeroThread.qlref b/java/ql/test/query-tests/ScheduledThreadPoolExecutorZeroThread/ScheduledThreadPoolExecutorZeroThread.qlref new file mode 100644 index 000000000000..e0089e4cf029 --- /dev/null +++ b/java/ql/test/query-tests/ScheduledThreadPoolExecutorZeroThread/ScheduledThreadPoolExecutorZeroThread.qlref @@ -0,0 +1,2 @@ +query: Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/java/ql/test/query-tests/ScheduledThreadPoolExecutorZeroThread/Test.java b/java/ql/test/query-tests/ScheduledThreadPoolExecutorZeroThread/Test.java new file mode 100644 index 000000000000..d02e6a3403eb --- /dev/null +++ b/java/ql/test/query-tests/ScheduledThreadPoolExecutorZeroThread/Test.java @@ -0,0 +1,11 @@ +import java.util.concurrent.ScheduledThreadPoolExecutor; + +public class Test { + void f() { + int i = 0; + ScheduledThreadPoolExecutor s = new ScheduledThreadPoolExecutor(1); // Compliant + ScheduledThreadPoolExecutor s1 = new ScheduledThreadPoolExecutor(0); // $ Alert + s.setCorePoolSize(0); // $ Alert + s.setCorePoolSize(i); // $ Alert + } +} From 1e0dd2a935514a25dcfa751ecbb60aa90d4657b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tam=C3=A1s=20Vajk?= Date: Thu, 26 Jun 2025 11:34:43 +0200 Subject: [PATCH 2/3] Apply suggestion from @michaelnebel Co-authored-by: Michael Nebel --- .../Concurrency/ScheduledThreadPoolExecutorZeroThread.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.md b/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.md index 428414b8f1aa..424407f5cc64 100644 --- a/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.md +++ b/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.md @@ -1,6 +1,6 @@ ## Overview -According the Java documentation on `ScheduledThreadPoolExecutor`, it is not a good idea to set `corePoolSize` to zero, since doing so indicates the executor to keep 0 threads in its pool and the executor will serve no purpose. +According to the Java documentation on `ScheduledThreadPoolExecutor`, it is not a good idea to set `corePoolSize` to zero, since doing so indicates the executor to keep 0 threads in its pool and the executor will serve no purpose. ## Recommendation From 1bd543a8a26ca7f727f9f1662db3b6775101d331 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Thu, 26 Jun 2025 11:36:32 +0200 Subject: [PATCH 3/3] Improve readability of the ID --- .../Concurrency/ScheduledThreadPoolExecutorZeroThread.ql | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.ql b/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.ql index cb6928a5b809..0b8acb5a0888 100644 --- a/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.ql +++ b/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.ql @@ -1,11 +1,12 @@ /** - * @id java/javautilconcurrentscheduledthreadpoolexecutor + * @id java/java-util-concurrent-scheduledthreadpoolexecutor * @name Zero threads set for `java.util.concurrent.ScheduledThreadPoolExecutor` * @description Setting `java.util.concurrent.ScheduledThreadPoolExecutor` to have 0 threads serves * no purpose and may indicate programmer error. * @kind problem * @precision very-high * @problem.severity recommendation + * @previous-id java/javautilconcurrentscheduledthreadpoolexecutor * @tags quality * reliability * correctness