FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Java: Add new quality query to detect `finalize` calls by jcogs33 · Pull Request #19075 · github/codeql · GitHub

Repository navigation

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension .expected  (2) .java  (1) .md  (1) .ql  (1) .qlref  (1) .qls  (1) All 6 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ ql/java/ql/src/Likely Bugs/Likely Typos/SuspiciousDateFormat.ql
ql/java/ql/src/Likely Bugs/Resource Leaks/CloseReader.ql
ql/java/ql/src/Likely Bugs/Resource Leaks/CloseWriter.ql
ql/java/ql/src/Performance/StringReplaceAllWithNonRegex.ql
ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DoNotCallFinalize.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
## Overview

Triggering garbage collection by directly calling `finalize()` may either have no effect or trigger unnecessary garbage collection, leading to erratic behavior, performance issues, or deadlock.

## Recommendation

Avoid calling `finalize()` in application code. Allow the JVM to determine a garbage collection schedule instead. If you need to explicitly release resources, provide a specific method to do so, such as by implementing the `AutoCloseable` interface and overriding its `close` method. You can then use a `try-with-resources` block to ensure that the resource is closed.

## Example

```java
class LocalCache {
private Collection<File> cacheFiles = ...;
// ...
}

void main() {
LocalCache cache = new LocalCache();
// ...
cache.finalize(); // NON_COMPLIANT
}

```

```java
import java.lang.AutoCloseable;
import java.lang.Override;

class LocalCache implements AutoCloseable {
private Collection<File> cacheFiles = ...;
// ...

@Override
public void close() throws Exception {
// release resources here if required
}
}

void main() {
// COMPLIANT: uses try-with-resources to ensure that
// a resource implementing AutoCloseable is closed.
try (LocalCache cache = new LocalCache()) {
// ...
}
}

```

# Implementation Notes

This rule ignores `super.finalize()` calls that occur within `finalize()` overrides since calling the superclass finalizer is required when overriding `finalize()`. Also, although overriding `finalize()` is not recommended, this rule only alerts on direct calls to `finalize()` and does not alert on method declarations overriding `finalize()`.

## References

- SEI CERT Oracle Coding Standard for Java: [MET12-J. Do not use finalizers](https://wiki.sei.cmu.edu/confluence/display/java/MET12-J.+Do+not+use+finalizers).
- Java API Specification: [Object.finalize()](https://docs.oracle.com/javase/10/docs/api/java/lang/Object.html#finalize()).
- Java API Specification: [Interface AutoCloseable](https://docs.oracle.com/javase/10/docs/api/java/lang/AutoCloseable.html).
- Java SE Documentation: [The try-with-resources Statement](https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html).
- Common Weakness Enumeration: [CWE-586](https://cwe.mitre.org/data/definitions/586).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/**
* @id java/do-not-call-finalize
* @previous-id java/do-not-use-finalizers
* @name Do not call `finalize()`
* @description Calling `finalize()` in application code may cause
* inconsistent program state or unpredictable behavior.
* @kind problem
* @precision high
* @problem.severity error
* @tags quality
* reliability
* correctness
* performance
* external/cwe/cwe-586
*/

import java

from MethodCall mc
where
mc.getMethod() instanceof FinalizeMethod and
// The Java documentation for `finalize()` states: "If a subclass overrides
// `finalize` it must invoke the superclass finalizer explicitly". Therefore,
// we do not alert on `super.finalize()` calls that occur within a callable
// that overrides `finalize`.
not exists(Callable caller, FinalizeMethod fm | caller = mc.getCaller() |
caller.(Method).overrides(fm) and
mc.getQualifier() instanceof SuperAccess
)
select mc, "Call to 'finalize()'."
3 changes: 2 additions & 1 deletion java/ql/src/codeql-suites/java-code-quality.qls
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
- include:
id:
- java/contradictory-type-checks
- java/do-not-call-finalize
- java/equals-on-unrelated-types
- java/inconsistent-equals-and-hashcode
- java/input-resource-leak
Expand All @@ -12,4 +13,4 @@
- java/suspicious-date-format
- java/type-variable-hides-type
- java/unchecked-cast-in-equals
- java/unused-container
- java/unused-container
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
| Test.java:4:9:4:23 | finalize(...) | Call to 'finalize()'. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
query: Violations of Best Practice/Undesirable Calls/DoNotCallFinalize.ql
postprocess: utils/test/InlineExpectationsTestQuery.ql
28 changes: 28 additions & 0 deletions java/ql/test/query-tests/DoNotCallFinalize/Test.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
public class Test {
void f() throws Throwable {
// NON_COMPLIANT
this.finalize(); // $ Alert
}

void f1() throws Throwable {
f(); // COMPLIANT
}

@Override
protected void finalize() throws Throwable {
// COMPLIANT: If a subclass overrides `finalize()`
// it must invoke the superclass finalizer explicitly.
super.finalize();
}

// Overload of `finalize`
protected void finalize(String s) throws Throwable {
// ...
}

void f2() throws Throwable {
// COMPLIANT: call to overload of `finalize`
this.finalize("overload");
}

}

Back | FazBrowse Home | New Git URL