Skip to content

fix: nullify last_propagated_event to avoid memory leak #16531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

Ocean-OS
Copy link
Member

Small fix to #16527— since last_propagated_event is never nullified, it could leak an event object.

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.
  • If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

  • Run the tests with pnpm test and lint the project with pnpm lint

Copy link

changeset-bot bot commented Jul 31, 2025

🦋 Changeset detected

Latest commit: 5d4e61e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

Playground

pnpm add https://pkg.pr.new/svelte@16531

@7nik
Copy link
Contributor

7nik commented Jul 31, 2025

Won't the issue return when we have multiple apps mounted one inside another?
It doesn't create a permanent leak, so it is ok as is.

@Ocean-OS
Copy link
Member Author

I see what you mean, thanks for pointing that out!

@Ocean-OS Ocean-OS closed this Jul 31, 2025
@paoloricciuti
Copy link
Member

We should also probably check what happens (i don't know if it can even happen) when you have multiple even listeners fire

@7nik
Copy link
Contributor

7nik commented Jul 31, 2025

I did this and it is super weird - sometimes one click results in dozens of runs.
Edit: Seems not unmounting the app has badly interfered with the result. Now it's better, but still multiple runs per click.

@Ocean-OS Ocean-OS deleted the nullify-last_propagated_event branch July 31, 2025 22:04
@WaiSiuKei
Copy link

I'm working on developing a complex application with a UI layer built using Svelte. This application is intended to be packaged and provided as a library for others to use, and it will be repeatedly initialized and destroyed. While checking for memory leaks in my application, I found that last_propagated_event is causing major interference. In the memory snapshots captured by Chrome DevTools, most of the data retained are references from last_propagated_event, which makes it difficult for me to quickly locate the information I need. On the other hand, after users initialize and then destroy the application, memory usage does not drop due to the lingering references from last_propagated_event, it negatively impacts the user experience.

@7nik
Copy link
Contributor

7nik commented Aug 12, 2025

But it isn't a constantly growing leak, right?

I confirm the underlying bug is fixed in Firefox 141.0.3, so I think we can revert this fix in one or two weeks.

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