From 7a2023b863f0dc036bd8ebe56e544d39b5403b3e Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Mon, 31 Mar 2025 21:31:23 -0400 Subject: [PATCH 1/6] Java: move original files --- ...RunMethodCalledOnJavaLangThreadDirectly.md | 58 +++++++++++ ...RunMethodCalledOnJavaLangThreadDirectly.ql | 51 ++++++++++ ...hodCalledOnJavaLangThreadDirectly.expected | 5 + ...MethodCalledOnJavaLangThreadDirectly.qlref | 1 + .../Test.java | 98 +++++++++++++++++++ 5 files changed, 213 insertions(+) create mode 100644 java/ql/src/Likely Bugs/Concurrency/RunMethodCalledOnJavaLangThreadDirectly.md create mode 100644 java/ql/src/Likely Bugs/Concurrency/RunMethodCalledOnJavaLangThreadDirectly.ql create mode 100644 java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/RunMethodCalledOnJavaLangThreadDirectly.expected create mode 100644 java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/RunMethodCalledOnJavaLangThreadDirectly.qlref create mode 100644 java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/Test.java diff --git a/java/ql/src/Likely Bugs/Concurrency/RunMethodCalledOnJavaLangThreadDirectly.md b/java/ql/src/Likely Bugs/Concurrency/RunMethodCalledOnJavaLangThreadDirectly.md new file mode 100644 index 000000000000..178b233468e4 --- /dev/null +++ b/java/ql/src/Likely Bugs/Concurrency/RunMethodCalledOnJavaLangThreadDirectly.md @@ -0,0 +1,58 @@ +# J-D-001: Method call to `java.lang.Thread` or its subclasses may indicate a logical bug + +Calling `run()` on `java.lang.Thread` or its subclasses may indicate misunderstanding on the programmer's part. + +## Overview + +The `java.lang` package provides class `Thread` for multithreading. This class provides a method named `start`, to begin executing its code in a separate thread, that calls another public API called `run`. However, directly calling `run` does not result in this multithreading behavior; rather, it executes the code in the context of the current thread. + +Meanwhile, the argument of the call to the constructor of `Thread` should implement `java.lang.Runnable` which provides the public method `run`. Calling this method directly also does not create a separate thread, however, this rule does not prohibit such calls. + +## Recommendation + +For instances of `Thread` and its subclasses, use `start` instead of `run` to start the thread and begin executing the `run` method of the `Runnable` instance used to construct the instance of `Thread`. + +## Example + +The following example creates an instance of `java.lang.Thread` and intends to execute it by calling the `run` method on it instead of `start`. + +```java +import java.lang.Thread; +import java.lang.Runnable; + +class Job implements Runnable { + public void run() { + /* ... */ + } +} + +class AnotherThread extends Thread { + AnotherThread(Runnable runnable) { + super(runnable); + } + + public void run() { + /* ... */ + } +} + +class ThreadExample { + public void f() { + Thread thread = new Thread(new Job()); + thread.run(); // NON_COMPLIANT + thread.start(); // COMPLIANT + + AnotherThread anotherThread = new AnotherThread(new Job()); + anotherThread.run(); // NON_COMPLIANT + anotherThread.start(); // COMPLIANT + + Job job = new Job(); + job.run(); // COMPLIANT + } +} +``` + +## References + +- Oracle: [Documentation of java.lang.Thread](https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html). +- Oracle: [Documentation of java.lang.Runnable](https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html). diff --git a/java/ql/src/Likely Bugs/Concurrency/RunMethodCalledOnJavaLangThreadDirectly.ql b/java/ql/src/Likely Bugs/Concurrency/RunMethodCalledOnJavaLangThreadDirectly.ql new file mode 100644 index 000000000000..1ee95bbc3b12 --- /dev/null +++ b/java/ql/src/Likely Bugs/Concurrency/RunMethodCalledOnJavaLangThreadDirectly.ql @@ -0,0 +1,51 @@ +/** + * @id java/run-method-called-on-java-lang-thread-directly + * @name J-D-001: Method call to `java.lang.Thread` or its subclasses may indicate a logical bug + * @description Calling `run()` on `java.lang.Thread` or its subclasses may indicate + * misunderstanding on the programmer's part. + * @kind problem + * @precision very-high + * @problem.severity warning + * @tags correctness + * performance + * concurrency + */ + +import java +import semmle.code.java.dataflow.DataFlow + +/** + * The import statement that brings java.lang.Thread into scope. + */ +class JavaLangThreadImport extends ImportType { + JavaLangThreadImport() { this.getImportedType().hasQualifiedName("java.lang", "Thread") } +} + +/** + * A class that inherits from `java.lang.Thread` either directly or indirectly. + */ +class JavaLangThreadSubclass extends Class { + JavaLangThreadSubclass() { + exists(JavaLangThreadImport javaLangThread | + this.getASupertype+() = javaLangThread.getImportedType() + ) + } +} + +class ProblematicRunMethodCall extends MethodCall { + ProblematicRunMethodCall() { + this.getMethod().getName() = "run" and + ( + exists(JavaLangThreadImport javaLangThread | + this.getQualifier().getType() = javaLangThread.getImportedType() + ) + or + exists(JavaLangThreadSubclass javaLangThreadSubclass | + this.getQualifier().getType() = javaLangThreadSubclass + ) + ) + } +} + +from ProblematicRunMethodCall problematicRunMethodCall +select problematicRunMethodCall, "The run method is called directly on a thread instance." diff --git a/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/RunMethodCalledOnJavaLangThreadDirectly.expected b/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/RunMethodCalledOnJavaLangThreadDirectly.expected new file mode 100644 index 000000000000..87c212e5918d --- /dev/null +++ b/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/RunMethodCalledOnJavaLangThreadDirectly.expected @@ -0,0 +1,5 @@ +| Test.java:70:5:70:16 | run(...) | The run method is called directly on a thread instance. | +| Test.java:74:5:74:24 | run(...) | The run method is called directly on a thread instance. | +| Test.java:78:5:78:24 | run(...) | The run method is called directly on a thread instance. | +| Test.java:82:5:82:27 | run(...) | The run method is called directly on a thread instance. | +| Test.java:86:5:86:27 | run(...) | The run method is called directly on a thread instance. | diff --git a/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/RunMethodCalledOnJavaLangThreadDirectly.qlref b/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/RunMethodCalledOnJavaLangThreadDirectly.qlref new file mode 100644 index 000000000000..92d09b7cba98 --- /dev/null +++ b/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/RunMethodCalledOnJavaLangThreadDirectly.qlref @@ -0,0 +1 @@ +rules/J-D-001/RunMethodCalledOnJavaLangThreadDirectly.ql diff --git a/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/Test.java b/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/Test.java new file mode 100644 index 000000000000..4e271dd771d4 --- /dev/null +++ b/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/Test.java @@ -0,0 +1,98 @@ +import java.lang.Thread; +import java.lang.Runnable; + +class Job implements Runnable { + public void run() { + /* ... */ + } +} + +/** + * A class that subclasses `java.lang.Thread` and inherits its `.run()` method. + */ +class AnotherThread1 extends Thread { + AnotherThread1(Runnable runnable) { + super(runnable); + } +} + +/** + * A class that directly subclasses `java.lang.Thread` and overrides its + * `.run()` method. + */ +class AnotherThread2 extends Thread { + AnotherThread2(Runnable runnable) { + super(runnable); + } + + /** + * An overriding definition of `Thread.run`. + */ + @Override + public void run() { + /* ... */ + } +} + +/** + * A class that indirectly subclasses `java.lang.Thread` by subclassing + * `AnotherThread1` and inherits its `.run()` + * method. + */ +class YetAnotherThread1 extends AnotherThread1 { + YetAnotherThread1(Runnable runnable) { + super(runnable); + } +} + +/** + * A class that indirectly subclasses `java.lang.Thread` by subclassing + * `AnotherThread2` and overrides its `.run()` + * method. + */ +class YetAnotherThread2 extends AnotherThread2 { + YetAnotherThread2(Runnable runnable) { + super(runnable); + } + + /** + * An overriding definition of `AnotherThread.run`. + */ + @Override + public void run() { + /* ... */ + } +} + +class ThreadExample { + public void f() { + Thread thread = new Thread(new Job()); + thread.run(); // NON_COMPLIANT: `Thread.run()` called directly. + thread.start(); // COMPLIANT: Thread started with `.start()`. + + AnotherThread1 anotherThread1 = new AnotherThread1(new Job()); + anotherThread1.run(); // NON_COMPLIANT: Inherited `Thread.run()` called on its instance. + anotherThread1.start(); // COMPLIANT: Inherited `Thread.start()` used to start the thread. + + AnotherThread2 anotherThread2 = new AnotherThread2(new Job()); + anotherThread2.run(); // NON_COMPLIANT: Overriden `Thread.run()` called on its instance. + anotherThread2.start(); // COMPLIANT: Overriden `Thread.start()` used to start the thread. + + YetAnotherThread1 yetAnotherThread1 = new YetAnotherThread1(new Job()); + yetAnotherThread1.run(); // NON_COMPLIANT: Inherited `AnotherThread1.run()` called on its instance. + yetAnotherThread1.start(); // COMPLIANT: Inherited `AnotherThread.start()` used to start the thread. + + YetAnotherThread2 yetAnotherThread2 = new YetAnotherThread2(new Job()); + yetAnotherThread2.run(); // NON_COMPLIANT: Overriden `AnotherThread2.run()` called on its instance. + yetAnotherThread2.start(); // COMPLIANT: Overriden `AnotherThread2.start()` used to start the thread. + + Runnable runnable = new Runnable() { + public void run() { + /* ... */ } + }; + runnable.run(); // COMPLIANT: This rule does not prevent `Runnable.run()` from being run. + + Job job = new Job(); + job.run(); // COMPLIANT: This rule does not prevent `Runnable.run()` from being run. + } +} From e266918871304c5dadea6b2b593601a6be89196c Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Thu, 3 Apr 2025 18:41:20 -0400 Subject: [PATCH 2/6] Java: add previous-id --- java/ql/src/Likely Bugs/Concurrency/CallsToRunnableRun.ql | 1 + 1 file changed, 1 insertion(+) diff --git a/java/ql/src/Likely Bugs/Concurrency/CallsToRunnableRun.ql b/java/ql/src/Likely Bugs/Concurrency/CallsToRunnableRun.ql index a6ff029557c1..9e4e1dd47143 100644 --- a/java/ql/src/Likely Bugs/Concurrency/CallsToRunnableRun.ql +++ b/java/ql/src/Likely Bugs/Concurrency/CallsToRunnableRun.ql @@ -6,6 +6,7 @@ * @problem.severity recommendation * @precision high * @id java/call-to-thread-run + * @previous-id java/run-method-called-on-java-lang-thread-directly * @tags quality * reliability * concurrency From 1172f82a4b00b7489f4f701d641de8e84b10e44f Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Fri, 4 Apr 2025 18:20:26 -0400 Subject: [PATCH 3/6] Java: update existing tests to inline expectations --- .../CallsToRunnableRun/CallsToRunnableRun.java | 14 +++++++------- .../CallsToRunnableRun/CallsToRunnableRun.qlref | 3 ++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/java/ql/test/query-tests/CallsToRunnableRun/CallsToRunnableRun.java b/java/ql/test/query-tests/CallsToRunnableRun/CallsToRunnableRun.java index 814c9d516bbf..258716e1fd43 100644 --- a/java/ql/test/query-tests/CallsToRunnableRun/CallsToRunnableRun.java +++ b/java/ql/test/query-tests/CallsToRunnableRun/CallsToRunnableRun.java @@ -1,18 +1,18 @@ import java.lang.Runnable; public class CallsToRunnableRun extends Thread implements Runnable{ - + private Thread wrapped; private Runnable callback; - + @Override public void run() { - wrapped.run(); - callback.run(); + wrapped.run(); // COMPLIANT: called within a `run` method + callback.run(); // COMPLIANT: called within a `run` method } - + public void bad() { - wrapped.run(); - callback.run(); + wrapped.run(); // $ Alert + callback.run(); // COMPLIANT: called on a `Runnable` object } } diff --git a/java/ql/test/query-tests/CallsToRunnableRun/CallsToRunnableRun.qlref b/java/ql/test/query-tests/CallsToRunnableRun/CallsToRunnableRun.qlref index fff4a4f5770c..4061770874fe 100644 --- a/java/ql/test/query-tests/CallsToRunnableRun/CallsToRunnableRun.qlref +++ b/java/ql/test/query-tests/CallsToRunnableRun/CallsToRunnableRun.qlref @@ -1 +1,2 @@ -Likely Bugs/Concurrency/CallsToRunnableRun.ql \ No newline at end of file +query: Likely Bugs/Concurrency/CallsToRunnableRun.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql From 12e7bbbae8977bf9e5a00fcf0f72e3c330a4d861 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Fri, 4 Apr 2025 18:57:44 -0400 Subject: [PATCH 4/6] Java: update existing tests to services tests --- .../CallsToRunnableRun.expected | 6 +- .../CallsToRunnableRun.java | 103 +++++++++++++++--- 2 files changed, 95 insertions(+), 14 deletions(-) diff --git a/java/ql/test/query-tests/CallsToRunnableRun/CallsToRunnableRun.expected b/java/ql/test/query-tests/CallsToRunnableRun/CallsToRunnableRun.expected index 0be8b917e026..7b8e175162ca 100644 --- a/java/ql/test/query-tests/CallsToRunnableRun/CallsToRunnableRun.expected +++ b/java/ql/test/query-tests/CallsToRunnableRun/CallsToRunnableRun.expected @@ -1 +1,5 @@ -| CallsToRunnableRun.java:15:3:15:15 | run(...) | Calling 'Thread.run()' rather than 'Thread.start()' will not spawn a new thread. | +| CallsToRunnableRun.java:67:5:67:16 | run(...) | Calling 'Thread.run()' rather than 'Thread.start()' will not spawn a new thread. | +| CallsToRunnableRun.java:71:5:71:24 | run(...) | Calling 'Thread.run()' rather than 'Thread.start()' will not spawn a new thread. | +| CallsToRunnableRun.java:75:5:75:24 | run(...) | Calling 'Thread.run()' rather than 'Thread.start()' will not spawn a new thread. | +| CallsToRunnableRun.java:79:5:79:27 | run(...) | Calling 'Thread.run()' rather than 'Thread.start()' will not spawn a new thread. | +| CallsToRunnableRun.java:83:5:83:27 | run(...) | Calling 'Thread.run()' rather than 'Thread.start()' will not spawn a new thread. | diff --git a/java/ql/test/query-tests/CallsToRunnableRun/CallsToRunnableRun.java b/java/ql/test/query-tests/CallsToRunnableRun/CallsToRunnableRun.java index 258716e1fd43..21ed4276458c 100644 --- a/java/ql/test/query-tests/CallsToRunnableRun/CallsToRunnableRun.java +++ b/java/ql/test/query-tests/CallsToRunnableRun/CallsToRunnableRun.java @@ -1,18 +1,95 @@ -import java.lang.Runnable; +class Job implements Runnable { + public void run() { + /* ... */ + } +} + +/** + * A class that subclasses `java.lang.Thread` and inherits its `.run()` method. + */ +class AnotherThread1 extends Thread { + AnotherThread1(Runnable runnable) { + super(runnable); + } +} + +/** + * A class that directly subclasses `java.lang.Thread` and overrides its + * `.run()` method. + */ +class AnotherThread2 extends Thread { + AnotherThread2(Runnable runnable) { + super(runnable); + } + + /** + * An overriding definition of `Thread.run`. + */ + @Override + public void run() { + super.run(); // COMPLIANT: called within a `run` method + } +} + +/** + * A class that indirectly subclasses `java.lang.Thread` by subclassing + * `AnotherThread1` and inherits its `.run()` + * method. + */ +class YetAnotherThread1 extends AnotherThread1 { + YetAnotherThread1(Runnable runnable) { + super(runnable); + } +} + +/** + * A class that indirectly subclasses `java.lang.Thread` by subclassing + * `AnotherThread2` and overrides its `.run()` + * method. + */ +class YetAnotherThread2 extends AnotherThread2 { + YetAnotherThread2(Runnable runnable) { + super(runnable); + } + + /** + * An overriding definition of `AnotherThread.run`. + */ + @Override + public void run() { + super.run(); // COMPLIANT: called within a `run` method + } +} + +class ThreadExample { + public void f() { + Thread thread = new Thread(new Job()); + thread.run(); // $ Alert - `Thread.run()` called directly. + thread.start(); // COMPLIANT: Thread started with `.start()`. + + AnotherThread1 anotherThread1 = new AnotherThread1(new Job()); + anotherThread1.run(); // $ Alert - Inherited `Thread.run()` called on its instance. + anotherThread1.start(); // COMPLIANT: Inherited `Thread.start()` used to start the thread. + + AnotherThread2 anotherThread2 = new AnotherThread2(new Job()); + anotherThread2.run(); // $ Alert - Overriden `Thread.run()` called on its instance. + anotherThread2.start(); // COMPLIANT: Overriden `Thread.start()` used to start the thread. -public class CallsToRunnableRun extends Thread implements Runnable{ + YetAnotherThread1 yetAnotherThread1 = new YetAnotherThread1(new Job()); + yetAnotherThread1.run(); // $ Alert - Inherited `AnotherThread1.run()` called on its instance. + yetAnotherThread1.start(); // COMPLIANT: Inherited `AnotherThread.start()` used to start the thread. - private Thread wrapped; - private Runnable callback; + YetAnotherThread2 yetAnotherThread2 = new YetAnotherThread2(new Job()); + yetAnotherThread2.run(); // $ Alert - Overriden `AnotherThread2.run()` called on its instance. + yetAnotherThread2.start(); // COMPLIANT: Overriden `AnotherThread2.start()` used to start the thread. - @Override - public void run() { - wrapped.run(); // COMPLIANT: called within a `run` method - callback.run(); // COMPLIANT: called within a `run` method - } + Runnable runnable = new Runnable() { + public void run() { + /* ... */ } + }; + runnable.run(); // COMPLIANT: called on `Runnable` object. - public void bad() { - wrapped.run(); // $ Alert - callback.run(); // COMPLIANT: called on a `Runnable` object - } + Job job = new Job(); + job.run(); // COMPLIANT: called on `Runnable` object. + } } From 87ab4d0160ea279f90b59087a31bc61bb06fb94a Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Fri, 4 Apr 2025 19:03:00 -0400 Subject: [PATCH 5/6] Java: remove java/run-method-called-on-java-lang-thread-directly using existing query java/call-to-thread-run instead --- ...RunMethodCalledOnJavaLangThreadDirectly.md | 58 ----------- ...RunMethodCalledOnJavaLangThreadDirectly.ql | 51 ---------- ...hodCalledOnJavaLangThreadDirectly.expected | 5 - ...MethodCalledOnJavaLangThreadDirectly.qlref | 1 - .../Test.java | 98 ------------------- 5 files changed, 213 deletions(-) delete mode 100644 java/ql/src/Likely Bugs/Concurrency/RunMethodCalledOnJavaLangThreadDirectly.md delete mode 100644 java/ql/src/Likely Bugs/Concurrency/RunMethodCalledOnJavaLangThreadDirectly.ql delete mode 100644 java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/RunMethodCalledOnJavaLangThreadDirectly.expected delete mode 100644 java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/RunMethodCalledOnJavaLangThreadDirectly.qlref delete mode 100644 java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/Test.java diff --git a/java/ql/src/Likely Bugs/Concurrency/RunMethodCalledOnJavaLangThreadDirectly.md b/java/ql/src/Likely Bugs/Concurrency/RunMethodCalledOnJavaLangThreadDirectly.md deleted file mode 100644 index 178b233468e4..000000000000 --- a/java/ql/src/Likely Bugs/Concurrency/RunMethodCalledOnJavaLangThreadDirectly.md +++ /dev/null @@ -1,58 +0,0 @@ -# J-D-001: Method call to `java.lang.Thread` or its subclasses may indicate a logical bug - -Calling `run()` on `java.lang.Thread` or its subclasses may indicate misunderstanding on the programmer's part. - -## Overview - -The `java.lang` package provides class `Thread` for multithreading. This class provides a method named `start`, to begin executing its code in a separate thread, that calls another public API called `run`. However, directly calling `run` does not result in this multithreading behavior; rather, it executes the code in the context of the current thread. - -Meanwhile, the argument of the call to the constructor of `Thread` should implement `java.lang.Runnable` which provides the public method `run`. Calling this method directly also does not create a separate thread, however, this rule does not prohibit such calls. - -## Recommendation - -For instances of `Thread` and its subclasses, use `start` instead of `run` to start the thread and begin executing the `run` method of the `Runnable` instance used to construct the instance of `Thread`. - -## Example - -The following example creates an instance of `java.lang.Thread` and intends to execute it by calling the `run` method on it instead of `start`. - -```java -import java.lang.Thread; -import java.lang.Runnable; - -class Job implements Runnable { - public void run() { - /* ... */ - } -} - -class AnotherThread extends Thread { - AnotherThread(Runnable runnable) { - super(runnable); - } - - public void run() { - /* ... */ - } -} - -class ThreadExample { - public void f() { - Thread thread = new Thread(new Job()); - thread.run(); // NON_COMPLIANT - thread.start(); // COMPLIANT - - AnotherThread anotherThread = new AnotherThread(new Job()); - anotherThread.run(); // NON_COMPLIANT - anotherThread.start(); // COMPLIANT - - Job job = new Job(); - job.run(); // COMPLIANT - } -} -``` - -## References - -- Oracle: [Documentation of java.lang.Thread](https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html). -- Oracle: [Documentation of java.lang.Runnable](https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html). diff --git a/java/ql/src/Likely Bugs/Concurrency/RunMethodCalledOnJavaLangThreadDirectly.ql b/java/ql/src/Likely Bugs/Concurrency/RunMethodCalledOnJavaLangThreadDirectly.ql deleted file mode 100644 index 1ee95bbc3b12..000000000000 --- a/java/ql/src/Likely Bugs/Concurrency/RunMethodCalledOnJavaLangThreadDirectly.ql +++ /dev/null @@ -1,51 +0,0 @@ -/** - * @id java/run-method-called-on-java-lang-thread-directly - * @name J-D-001: Method call to `java.lang.Thread` or its subclasses may indicate a logical bug - * @description Calling `run()` on `java.lang.Thread` or its subclasses may indicate - * misunderstanding on the programmer's part. - * @kind problem - * @precision very-high - * @problem.severity warning - * @tags correctness - * performance - * concurrency - */ - -import java -import semmle.code.java.dataflow.DataFlow - -/** - * The import statement that brings java.lang.Thread into scope. - */ -class JavaLangThreadImport extends ImportType { - JavaLangThreadImport() { this.getImportedType().hasQualifiedName("java.lang", "Thread") } -} - -/** - * A class that inherits from `java.lang.Thread` either directly or indirectly. - */ -class JavaLangThreadSubclass extends Class { - JavaLangThreadSubclass() { - exists(JavaLangThreadImport javaLangThread | - this.getASupertype+() = javaLangThread.getImportedType() - ) - } -} - -class ProblematicRunMethodCall extends MethodCall { - ProblematicRunMethodCall() { - this.getMethod().getName() = "run" and - ( - exists(JavaLangThreadImport javaLangThread | - this.getQualifier().getType() = javaLangThread.getImportedType() - ) - or - exists(JavaLangThreadSubclass javaLangThreadSubclass | - this.getQualifier().getType() = javaLangThreadSubclass - ) - ) - } -} - -from ProblematicRunMethodCall problematicRunMethodCall -select problematicRunMethodCall, "The run method is called directly on a thread instance." diff --git a/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/RunMethodCalledOnJavaLangThreadDirectly.expected b/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/RunMethodCalledOnJavaLangThreadDirectly.expected deleted file mode 100644 index 87c212e5918d..000000000000 --- a/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/RunMethodCalledOnJavaLangThreadDirectly.expected +++ /dev/null @@ -1,5 +0,0 @@ -| Test.java:70:5:70:16 | run(...) | The run method is called directly on a thread instance. | -| Test.java:74:5:74:24 | run(...) | The run method is called directly on a thread instance. | -| Test.java:78:5:78:24 | run(...) | The run method is called directly on a thread instance. | -| Test.java:82:5:82:27 | run(...) | The run method is called directly on a thread instance. | -| Test.java:86:5:86:27 | run(...) | The run method is called directly on a thread instance. | diff --git a/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/RunMethodCalledOnJavaLangThreadDirectly.qlref b/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/RunMethodCalledOnJavaLangThreadDirectly.qlref deleted file mode 100644 index 92d09b7cba98..000000000000 --- a/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/RunMethodCalledOnJavaLangThreadDirectly.qlref +++ /dev/null @@ -1 +0,0 @@ -rules/J-D-001/RunMethodCalledOnJavaLangThreadDirectly.ql diff --git a/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/Test.java b/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/Test.java deleted file mode 100644 index 4e271dd771d4..000000000000 --- a/java/ql/test/query-tests/RunMethodCalledOnJavaLangThreadDirectly/Test.java +++ /dev/null @@ -1,98 +0,0 @@ -import java.lang.Thread; -import java.lang.Runnable; - -class Job implements Runnable { - public void run() { - /* ... */ - } -} - -/** - * A class that subclasses `java.lang.Thread` and inherits its `.run()` method. - */ -class AnotherThread1 extends Thread { - AnotherThread1(Runnable runnable) { - super(runnable); - } -} - -/** - * A class that directly subclasses `java.lang.Thread` and overrides its - * `.run()` method. - */ -class AnotherThread2 extends Thread { - AnotherThread2(Runnable runnable) { - super(runnable); - } - - /** - * An overriding definition of `Thread.run`. - */ - @Override - public void run() { - /* ... */ - } -} - -/** - * A class that indirectly subclasses `java.lang.Thread` by subclassing - * `AnotherThread1` and inherits its `.run()` - * method. - */ -class YetAnotherThread1 extends AnotherThread1 { - YetAnotherThread1(Runnable runnable) { - super(runnable); - } -} - -/** - * A class that indirectly subclasses `java.lang.Thread` by subclassing - * `AnotherThread2` and overrides its `.run()` - * method. - */ -class YetAnotherThread2 extends AnotherThread2 { - YetAnotherThread2(Runnable runnable) { - super(runnable); - } - - /** - * An overriding definition of `AnotherThread.run`. - */ - @Override - public void run() { - /* ... */ - } -} - -class ThreadExample { - public void f() { - Thread thread = new Thread(new Job()); - thread.run(); // NON_COMPLIANT: `Thread.run()` called directly. - thread.start(); // COMPLIANT: Thread started with `.start()`. - - AnotherThread1 anotherThread1 = new AnotherThread1(new Job()); - anotherThread1.run(); // NON_COMPLIANT: Inherited `Thread.run()` called on its instance. - anotherThread1.start(); // COMPLIANT: Inherited `Thread.start()` used to start the thread. - - AnotherThread2 anotherThread2 = new AnotherThread2(new Job()); - anotherThread2.run(); // NON_COMPLIANT: Overriden `Thread.run()` called on its instance. - anotherThread2.start(); // COMPLIANT: Overriden `Thread.start()` used to start the thread. - - YetAnotherThread1 yetAnotherThread1 = new YetAnotherThread1(new Job()); - yetAnotherThread1.run(); // NON_COMPLIANT: Inherited `AnotherThread1.run()` called on its instance. - yetAnotherThread1.start(); // COMPLIANT: Inherited `AnotherThread.start()` used to start the thread. - - YetAnotherThread2 yetAnotherThread2 = new YetAnotherThread2(new Job()); - yetAnotherThread2.run(); // NON_COMPLIANT: Overriden `AnotherThread2.run()` called on its instance. - yetAnotherThread2.start(); // COMPLIANT: Overriden `AnotherThread2.start()` used to start the thread. - - Runnable runnable = new Runnable() { - public void run() { - /* ... */ } - }; - runnable.run(); // COMPLIANT: This rule does not prevent `Runnable.run()` from being run. - - Job job = new Job(); - job.run(); // COMPLIANT: This rule does not prevent `Runnable.run()` from being run. - } -} From 42904113b4e398aa6cf206775c76f1d329bfda84 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Sun, 29 Jun 2025 22:50:10 -0400 Subject: [PATCH 6/6] Java: add qhelp references --- .../src/Likely Bugs/Concurrency/CallsToRunnableRun.qhelp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/java/ql/src/Likely Bugs/Concurrency/CallsToRunnableRun.qhelp b/java/ql/src/Likely Bugs/Concurrency/CallsToRunnableRun.qhelp index b6cb3a730876..a97c17c13188 100644 --- a/java/ql/src/Likely Bugs/Concurrency/CallsToRunnableRun.qhelp +++ b/java/ql/src/Likely Bugs/Concurrency/CallsToRunnableRun.qhelp @@ -49,6 +49,15 @@ continue while the child thread is waiting, so that "Main thread activity" is pr
  • The Java Tutorials: Defining and Starting a Thread.
    • +
  • + SEI CERT Oracle Coding Standard for Java: THI00-J. Do not invoke Thread.run(). +
    • +
  • + Java API Specification: Thread. +
    • +
  • + Java API Specification: Runnable. +