Skip to content

Conversation

@atharrva01
Copy link

Summary

Fixes a memory leak caused by push-only subscriptions in Phoenix when navigating between experiments. Route-scoped Angular components subscribed to long-lived singleton services without an unsubscribe mechanism, causing callbacks to accumulate and destroyed components to remain in memory.


Issue Description

Core services (EventDisplay, ActiveVariable) stored callbacks in internal arrays but did not provide a way to remove them. When Angular components were destroyed on navigation, their callbacks remained registered and retained component instances via closures.

// Before
this.onEventsChange.push(callback);

Over time this led to unbounded memory growth and increasing event dispatch cost.


Steps to Reproduce

  1. Open Phoenix and DevTools → Memory
  2. Take a heap snapshot
  3. Navigate repeatedly between experiments (ATLAS → CMS → LHCb)
  4. Take another snapshot

Before fix: callback counts and retained component instances grow with each navigation.


Fix

Subscription APIs now return an unsubscribe function, and Angular components clean up subscriptions on destroy.

// Core API
const unsubscribe = listenToLoadedEventsChange(cb);
// Component
ngOnDestroy() {
  unsubscribe();
}

This makes subscriptions lifecycle-safe and allows destroyed components to be garbage-collected.


Impact

  • Callback counts remain stable across navigation
  • Destroyed components are released
  • Memory usage stays flat over long sessions
  • No behavior change for existing consumers

Backward Compatibility

Fully backward-compatible. Existing callers that ignore the returned unsubscribe function continue to work unchanged.


@atharrva01
Copy link
Author

Hi @EdwardMoyse 👋,
This PR fixes a memory leak caused by persistent subscriptions retaining destroyed components across navigation. Subscriptions are now lifecycle-safe and cleaned up on destroy.

👍 Would appreciate a review whenever you get time. Thanks!

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.

1 participant