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]

Code Review ✅ Approved 1 closed / 1 findings

Clears the safeSymbols cache on each file scan to prevent memory leaks in long-lived processes. The fix addresses the issue where symbols accumulated across files, and now follows the same pattern as the existing ifStatementCache and firstNullCheckCache clearing logic.

✅ 1 closed

✅ 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.

Review coverage

🧪 Functional validation No results

📋 Rules No rules evaluated

🤖 Auto-approval Not enabled · Set up

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

[leveretka]

The code looks fine. Did you manage to reproduce the leak and test locally that it doesn't occur anymore?

You can run the analysis locally and connect to the process with VisualVM and get a heap dump before and after the fix. This way you'll be able to not just say that you fixed it but You could also estimate the impact on a real project. (You can try analyzing SonarJava itself at least)

And another question, don't you need an approval from someone from the CQ team? do you think mine would be enough here?

[romainbrenguier]

@leveretka
On sonar-java at least, it doesn't seem to have an impact on memory used. Do you have an example project where you know the memory leak is visible?

Before the fix:

After the fix:

[leveretka]

@romainbrenguier I'm not sure I can answer the question about the leak based on the charts solely. Of course the retained memory highly depends on the actual usage of Optional.map in the repo.

What I'd check is if 'safeSymbols' is growing till the end of analysis by generating heapdumps during the analysis. And see if it contains symbols from the old files and references old files ASTs. Of course at the end of the analysis everything is cleaned.

Also, I believe it will more affect single huge modules.

Another possible issur is that the source files are still there due to the other leak. But that requires looking at the heapdump between the files.

[romainbrenguier]

@romainbrenguier I'm not sure I can answer the question about the leak based on the charts solely. Of course the retained memory highly depends on the actual usage of Optional.map in the repo.

What I'd check is if 'safeSymbols' is growing till the end of analysis by generating heapdumps during the analysis. And see if it contains symbols from the old files and references old files ASTs. Of course at the end of the analysis everything is cleaned.

Also, I believe it will more affect single huge modules.

Another possible issur is that the source files are still there due to the other leak. But that requires looking at the heapdump between the files.

Looking at the heap dump during the analizis of sonar-java, the safeSymbols map always have 0 elements, so it's not a good example to test the memory leak.

[romainbrenguier]

I don't have a good example to reproduce the leak for now, but I'm going to merge anyway since it can only improve things.