Skip to content

Java: update java/call-to-thread-run #19175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Jun 30, 2025
Merged

Java: update java/call-to-thread-run #19175

merged 6 commits into from
Jun 30, 2025

Conversation

jcogs33
Copy link
Contributor

@jcogs33 jcogs33 commented Apr 1, 2025 •
edited
Loading

Description

Minor updates to the pre-existing java/call-to-thread-run quality query based on the similar java/run-method-called-on-java-lang-thread-directly query from the services team's quality queries.

Specifically:

  • Adds @previous-id to reference the services query
  • Updates tests to inline expectations and adds the tests from the services query
  • Adds some qhelp references

Consideration

Changes from the services team's query. Let me know if you disagree with any of these changes:

  • Left the precision/severity as-is in the existing query (high/recommendation).
  • Did not include the performance tag from the services query.
  • Left the exclusion for calls to run within a run method from the existing query. This exclusion was not in the services team's query.
  • Did not keep the reliance on the existence of a java.lang.Thread import statement from the services team's query. Since java.lang classes do not need to be explicitly imported, the reliance on the existence of this import statement was causing FNs.

Alert count notes:

  • Alert count comparison between the two queries. The differences are mainly caused by the FNs from the java.lang.Thread import statement requirement.
    • MRVA top-100:
      • java/run-method-called-on-java-lang-thread-directly: 8 alerts
      • java/call-to-thread-run: 62 alerts
    • MRVA top-1000:
      • java/run-method-called-on-java-lang-thread-directly: 18 alerts
      • java/call-to-thread-run: 161 alerts

Other Notes:

  • Autofixes look reasonable. Replaces calls to run() with start() as recommended by the qhelp.

@jcogs33 jcogs33 force-pushed the jcogs33/java/call-to-thread-run branch from 72e089f to 67b93dd Compare April 5, 2025 00:19
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 5, 2025 •
edited
Loading

QHelp previews:

java/ql/src/Likely Bugs/Concurrency/CallsToRunnableRun.qhelp

Direct call to a run() method

A direct call of a Thread object's run method does not start a separate thread. The method is executed within the current thread. This is an unusual use because Thread.run() is normally intended to be called from within a separate thread.

Recommendation

To execute Runnable.run from within a separate thread, do one of the following:

  • Construct a Thread object using the Runnable object, and call start on the Thread object.
  • Define a subclass of a Thread object, and override the definition of its run method. Then construct an instance of this subclass and call start on that instance directly.

Example

In the following example, the main thread, ThreadDemo, calls the child thread, NewThread, using run. This causes the child thread to run to completion before the rest of the main thread is executed, so that "Child thread activity" is printed before "Main thread activity".

public class ThreadDemo {
    public static void main(String args[]) {
        NewThread runnable = new NewThread();

        runnable.run();    // Call to 'run' does not start a separate thread

        System.out.println("Main thread activity.");
    }
}

class NewThread extends Thread {
    public void run() {
        try {
            Thread.sleep(10000);
        }
        catch (InterruptedException e) {
            System.out.println("Child interrupted.");
        }
        System.out.println("Child thread activity.");
    }
}

To enable the two threads to run concurrently, create the child thread and call start, as shown below. This causes the main thread to continue while the child thread is waiting, so that "Main thread activity" is printed before "Child thread activity".

public class ThreadDemo {
    public static void main(String args[]) {
    	NewThread runnable = new NewThread();
    	
        runnable.start();                                         // Call 'start' method
        
        System.out.println("Main thread activity.");
    }
}

References

@jcogs33 jcogs33 force-pushed the jcogs33/java/call-to-thread-run branch from 67b93dd to 3866cfc Compare April 5, 2025 00:30
@jcogs33 jcogs33 force-pushed the jcogs33/java/call-to-thread-run branch from 3866cfc to a0bb0e2 Compare June 30, 2025 02:22
@jcogs33 jcogs33 force-pushed the jcogs33/java/call-to-thread-run branch from a0bb0e2 to 4290411 Compare June 30, 2025 02:51
@jcogs33 jcogs33 added the no-change-note-required This PR does not need a change note label Jun 30, 2025
@jcogs33 jcogs33 marked this pull request as ready for review June 30, 2025 03:00
@jcogs33 jcogs33 requested a review from a team as a code owner June 30, 2025 03:00
@jcogs33
Copy link
Contributor Author

jcogs33 commented Jun 30, 2025

cc @knewbury01

@jcogs33 jcogs33 changed the title Java: Add new quality query to detect calls to Thread.run() Java: update java/call-to-thread-run Jun 30, 2025
@jcogs33 jcogs33 merged commit de09122 into github:main Jun 30, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@owen-mc owen-mc owen-mc approved these changes

Assignees
No one assigned
Labels
documentation Java no-change-note-required This PR does not need a change note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jcogs33 @owen-mc