mirror of
https://github.com/github/codeql.git
synced 2026-04-25 00:35:20 +02:00
Java: remove java/run-method-called-on-java-lang-thread-directly
using existing query java/call-to-thread-run instead
This commit is contained in:
Jami Cogswell
@@ -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).
|
||||
@@ -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."
|
||||
@@ -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. |
|
||||
@@ -1 +0,0 @@
|
||||
rules/J-D-001/RunMethodCalledOnJavaLangThreadDirectly.ql
|
||||
@@ -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.
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user