SONARJAVA-6972 fix: clear safeSymbols to prevent memory leak

[romainbrenguier]

Problem

The safeSymbols field in BoxedBooleanExpressionsCheck accumulates symbols across file scans but is never cleared. This causes a memory leak when the analyzer processes multiple files, as the set continues to grow without bound.

The two static caches (ifStatementCache and firstNullCheckCache) are properly cleared at the start of each file scan, but safeSymbols was overlooked.

Solution

Add safeSymbols.clear() in scanFile() alongside the existing cache clear calls. This ensures the set is reset for each file while maintaining correct behavior within a single file.

Verification

• Existing tests pass
• The fix follows the same pattern used for the other caches in the same method

[hashicorp-vault-sonar-prod]

SONARJAVA-6972

[gitar-bot]

💡 Performance: Caches cleared only before scan, so last file's AST stays retained

The clear calls (including the new safeSymbols.clear()) run at the start of scanFile, so after the final file of the analysis is scanned, ifStatementCache still maps every ancestor Tree of each null-check usage (populated in getParentConditionalBranch, line 246) and firstNullCheckCache still holds that file's Symbols. Because those two maps are static, that data stays strongly reachable for the lifetime of the plugin classloader — it is not released when the check instance and VisitorsBridge are dropped — which keeps a whole compilation unit's tree alive in long-lived processes (SonarLint/IDE sessions), contradicting the PR description's claim that the two static caches are "properly cleared". Clearing at the end of the scan (or via the EndOfAnalysis hook, whose javadoc explicitly warns that "keeping state between files can lead to memory leaks") releases the memory instead of merely bounding it to one file.

Clear the caches after the scan in a finally block so nothing is retained once the file (and the analysis) is done.:

@Override
public void scanFile(JavaFileScannerContext context) {
  this.context = context;
  try {
    if (context.getSemanticModel() != null) {
      scan(context.getTree());
    }
  } finally {
    ifStatementCache.clear();
    firstNullCheckCache.clear();
    safeSymbols.clear();
  }
}

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Fixes memory leak by clearing safeSymbols at the start of each file scan, following the pattern used for other static caches. Consider also clearing caches at the end of analysis rather than at the start of the next scan, so the final file's AST is not retained for the lifetime of the plugin classloader in long-lived processes like IDE sessions.

💡 Performance: Caches cleared only before scan, so last file's AST stays retained

📄 java-checks/src/main/java/org/sonar/java/checks/BoxedBooleanExpressionsCheck.java:81-82 📄 java-checks/src/main/java/org/sonar/java/checks/BoxedBooleanExpressionsCheck.java:90-98 📄 java-checks/src/main/java/org/sonar/java/checks/BoxedBooleanExpressionsCheck.java:225 📄 java-checks/src/main/java/org/sonar/java/checks/BoxedBooleanExpressionsCheck.java:246

The clear calls (including the new safeSymbols.clear()) run at the start of scanFile, so after the final file of the analysis is scanned, ifStatementCache still maps every ancestor Tree of each null-check usage (populated in getParentConditionalBranch, line 246) and firstNullCheckCache still holds that file's Symbols. Because those two maps are static, that data stays strongly reachable for the lifetime of the plugin classloader — it is not released when the check instance and VisitorsBridge are dropped — which keeps a whole compilation unit's tree alive in long-lived processes (SonarLint/IDE sessions), contradicting the PR description's claim that the two static caches are "properly cleared". Clearing at the end of the scan (or via the EndOfAnalysis hook, whose javadoc explicitly warns that "keeping state between files can lead to memory leaks") releases the memory instead of merely bounding it to one file.

Clear the caches after the scan in a finally block so nothing is retained once the file (and the analysis) is done.

@Override
public void scanFile(JavaFileScannerContext context) {
  this.context = context;
  try {
    if (context.getSemanticModel() != null) {
      scan(context.getTree());
    }
  } finally {
    ifStatementCache.clear();
    firstNullCheckCache.clear();
    safeSymbols.clear();
  }
}

🤖 Prompt for agents

Code Review: Fixes memory leak by clearing `safeSymbols` at the start of each file scan, following the pattern used for other static caches. Consider also clearing caches at the end of analysis rather than at the start of the next scan, so the final file's AST is not retained for the lifetime of the plugin classloader in long-lived processes like IDE sessions.

1. 💡 Performance: Caches cleared only before scan, so last file's AST stays retained
   Files: java-checks/src/main/java/org/sonar/java/checks/BoxedBooleanExpressionsCheck.java:81-82, java-checks/src/main/java/org/sonar/java/checks/BoxedBooleanExpressionsCheck.java:90-98, java-checks/src/main/java/org/sonar/java/checks/BoxedBooleanExpressionsCheck.java:225, java-checks/src/main/java/org/sonar/java/checks/BoxedBooleanExpressionsCheck.java:246

   The clear calls (including the new `safeSymbols.clear()`) run at the *start* of `scanFile`, so after the final file of the analysis is scanned, `ifStatementCache` still maps every ancestor `Tree` of each null-check usage (populated in `getParentConditionalBranch`, line 246) and `firstNullCheckCache` still holds that file's `Symbol`s. Because those two maps are `static`, that data stays strongly reachable for the lifetime of the plugin classloader — it is not released when the check instance and `VisitorsBridge` are dropped — which keeps a whole compilation unit's tree alive in long-lived processes (SonarLint/IDE sessions), contradicting the PR description's claim that the two static caches are "properly cleared". Clearing at the end of the scan (or via the `EndOfAnalysis` hook, whose javadoc explicitly warns that "keeping state between files can lead to memory leaks") releases the memory instead of merely bounding it to one file.

   Fix (Clear the caches after the scan in a finally block so nothing is retained once the file (and the analysis) is done.):
   @Override
   public void scanFile(JavaFileScannerContext context) {
     this.context = context;
     try {
       if (context.getSemanticModel() != null) {
         scan(context.getTree());
       }
     } finally {
       ifStatementCache.clear();
       firstNullCheckCache.clear();
       safeSymbols.clear();
     }
   }

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube