Skip to content

Remove listeners in MinimalState if bundle is stopped#1712

Merged
HannesWell merged 1 commit intoeclipse-pde:masterfrom
ptziegler:cleanup-listeners-in-minimal-state
Apr 4, 2025
Merged

Remove listeners in MinimalState if bundle is stopped#1712
HannesWell merged 1 commit intoeclipse-pde:masterfrom
ptziegler:cleanup-listeners-in-minimal-state

Conversation

@ptziegler
Copy link
Copy Markdown
Contributor

This change adapts the listeners that have been added with 24e9bb5 so that they are removed again, once the bundle is shut down and thus makes the classloader eligible for garbage collection.

@ptziegler
Copy link
Copy Markdown
Contributor Author

One of the other memory leaks in PDE I've noticed:
image

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 1, 2025

Test Results

   285 files  +  6     285 suites  +6   48m 48s ⏱️ + 2m 10s
 3 608 tests ±  0   3 532 ✅ +  1   76 💤 ± 0  0 ❌  - 1 
11 016 runs  +194  10 785 ✅ +165  231 💤 +30  0 ❌  - 1 

Results for commit 12beb8a. ± Comparison against base commit ce6d9c2.

♻️ This comment has been updated with latest results.

Copy link
Copy Markdown
Member

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

Thank you for this enhancement. Technically it looks good, I just have a few code-style nit-picks below.

Comment thread ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/MinimalState.java Outdated
Comment thread ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/MinimalState.java Outdated
This change adapts the listeners that have been added with
24e9bb5 so that they are removed again,
once the bundle is shut down and thus makes the classloader eligible for
garbage collection.
@HannesWell HannesWell force-pushed the cleanup-listeners-in-minimal-state branch from 5c89aec to 12beb8a Compare April 4, 2025 21:19
Copy link
Copy Markdown
Member

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

I have now applied the requested changes to this PR.

Thank you for this contribution!

@HannesWell HannesWell merged commit 14f66ae into eclipse-pde:master Apr 4, 2025
19 checks passed
@ptziegler
Copy link
Copy Markdown
Contributor Author

Whoops. Apologies, I didn't see you comment. Thanks for applying the the changes and merging everything.

@iloveeclipse
Copy link
Copy Markdown
Member

This caused regression, please check #1714

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.

3 participants