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. +
  • 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 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 814c9d516bbf..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; - -public class CallsToRunnableRun extends Thread implements Runnable{ - - private Thread wrapped; - private Runnable callback; - - @Override - public void run() { - wrapped.run(); - callback.run(); - } - - public void bad() { - wrapped.run(); - callback.run(); - } +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. + + 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. + + 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. + + Runnable runnable = new Runnable() { + public void run() { + /* ... */ } + }; + runnable.run(); // COMPLIANT: called on `Runnable` object. + + Job job = new Job(); + job.run(); // COMPLIANT: called on `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