Skip to content

IEP-1724: SWT resource leaks and disposal crashes in IDF Size Analysis Editor#1410

Merged
kolipakakondal merged 1 commit into
masterfrom
IEP-1724
Mar 17, 2026
Merged

IEP-1724: SWT resource leaks and disposal crashes in IDF Size Analysis Editor#1410
kolipakakondal merged 1 commit into
masterfrom
IEP-1724

Conversation

@sigmaaa
Copy link
Copy Markdown
Collaborator

@sigmaaa sigmaaa commented Mar 7, 2026

Description

During testing of the application size analysis, I noticed some SWT exceptions and discovered resource leaks that are addressed in this PR.

Fixes # (IEP-1724)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How has this been tested?

Open Activity Monitor and run the application size analysis. Go to Charts and click “Switch to …”. The error log should contain no SWT exceptions, and there should be no significant increase in memory usage.

Test Configuration:

  • ESP-IDF Version:
  • OS (Windows,Linux and macOS):

Dependent components impacted by this PR:

  • Application Size Analysis Editor

Checklist

  • PR Self Reviewed
  • Applied Code formatting
  • Added Documentation
  • Added Unit Test
  • Verified on all platforms - Windows,Linux and macOS

Summary by CodeRabbit

  • Bug Fixes
    • Improved resource management in the Size Analysis feature by ensuring UI components are properly cleaned up when no longer needed, preventing memory leaks and enhancing application stability.

@sigmaaa sigmaaa self-assigned this Mar 7, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 7, 2026

📝 Walkthrough

Walkthrough

These changes add explicit resource disposal logic to three SWT UI editor components in the size analysis UI package. Fonts, images, and editor instances are now properly disposed when their parent widgets are destroyed to prevent resource leaks.

Changes

Cohort / File(s) Summary
Resource Disposal in UI Components
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/size/IDFSizeAnalysisEditor.java, IDFSizeChartsComposite.java, IDFSizeOverviewComposite.java
Adds explicit disposal of SWT resources: boldFont disposed via listener in IDFSizeAnalysisEditor and IDFSizeOverviewComposite; image disposal listener added in IDFSizeChartsComposite. IDFSizeOverviewComposite introduces tableEditors collection to track and dispose TableEditor instances. Guard checks added to prevent operations on null or disposed widgets.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • kolipakakondal
  • AndriiFilippov
  • alirana01

Poem

🐰✨ A rabbit hops through widgets with care,
Disposing fonts and images everywhere,
No leaks shall escape our keen eye,
Resources freed as components say goodbye!
Memory clean, the UI runs light—
Our SWT cleanup done just right! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: fixing SWT resource leaks and disposal crashes in the IDF Size Analysis Editor, which directly corresponds to the file-level changes addressing font, image, and TableEditor disposal.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch IEP-1724

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/size/IDFSizeChartsComposite.java (1)

318-323: Correct disposal pattern with minor redundancy.

The dispose listener properly ensures the SWT Image is released when the Label is disposed. The !swtImg.isDisposed() check is good practice.

Minor: The swtImg != null check is redundant since swtImg is effectively final and was successfully created at line 315 before this listener is registered—it can never be null when the lambda executes.

🔧 Optional: Remove redundant null check
 			imgLabel.addDisposeListener(e -> {
-				if (swtImg != null && !swtImg.isDisposed())
+				if (!swtImg.isDisposed())
 				{
 					swtImg.dispose();
 				}
 			});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/size/IDFSizeChartsComposite.java`
around lines 318 - 323, The dispose listener on imgLabel uses a redundant null
check for swtImg; since swtImg is created before registering the listener it
cannot be null when invoked, so simplify the lambda in
imgLabel.addDisposeListener(...) to only check !swtImg.isDisposed() before
calling swtImg.dispose(); remove the swtImg != null branch to reduce dead code
while keeping the isDisposed guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/size/IDFSizeChartsComposite.java`:
- Around line 318-323: The dispose listener on imgLabel uses a redundant null
check for swtImg; since swtImg is created before registering the listener it
cannot be null when invoked, so simplify the lambda in
imgLabel.addDisposeListener(...) to only check !swtImg.isDisposed() before
calling swtImg.dispose(); remove the swtImg != null branch to reduce dead code
while keeping the isDisposed guard.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f7d59c0b-1ed0-403d-9e17-edacca3e6eb8

📥 Commits

Reviewing files that changed from the base of the PR and between 2261868 and 52ec136.

📒 Files selected for processing (3)
  • bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/size/IDFSizeAnalysisEditor.java
  • bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/size/IDFSizeChartsComposite.java
  • bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/size/IDFSizeOverviewComposite.java

Copy link
Copy Markdown
Collaborator

@kolipakakondal kolipakakondal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown
Collaborator

@alirana01 alirana01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sigmaaa this is fine but maybe add a small dispose listener that can be reused on other places as well a bit generic and maybe just one instance of that will be fine for every swt component but that can be done in other PR

@AndriiFilippov
Copy link
Copy Markdown
Collaborator

LGTM 👍

@kolipakakondal kolipakakondal merged commit a048a14 into master Mar 17, 2026
11 of 12 checks passed
@sigmaaa sigmaaa added this to the v4.2.0 milestone Mar 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants