Skip to content

Conversation

@Sanchit2662
Copy link

🛠 Fix: Prevent Event Listener Accumulation on Section Navigation

Impact

Phoenix uses a root-level EventDisplayService that stays alive while navigating between sections (ATLAS, CMS, LHCb, TrackML, etc.).
Each section calls eventDisplay.init() on navigation, which caused global event listeners to be re-registered every time without removing the old ones.

Over multiple navigations, this led to:

  • Growing numbers of resize, keydown, and OrbitControls listeners
  • Noticeable performance degradation
  • Window resize becoming sluggish
  • Keyboard shortcuts triggering multiple times
  • Old Three.js objects being retained in memory

This issue directly affects normal Phoenix usage, especially when users compare multiple detectors in a single session.


Steps to Reproduce

  1. Open Phoenix in Chrome and open DevTools → Console

  2. Navigate through several sections:

    Home → ATLAS → CMS → LHCb → TrackML → Playground
    
  3. After each navigation, run:

    getEventListeners(window).resize.length

Before the fix:
The listener count increases on every navigation.

Expected behavior:
Listener count should remain constant.


Root Cause

  • EventDisplayService is a singleton (providedIn: 'root')
  • init() is called on every section navigation
  • init() assumes a fresh instance and registers global listeners
  • Listeners were added using anonymous callbacks, so they could not be removed
  • No cleanup occurred before re-initialization

This resulted in unbounded listener accumulation over time.


Fix

The fix introduces explicit lifecycle cleanup and makes re-initialization safe:

  • EventDisplay now cleans up before re-initializing
  • Global event listeners are stored as class-level references
  • Existing listeners are removed before adding new ones
  • Cleanup methods ensure no stale listeners remain

Minimal example:

if (this.isInitialized) {
  this.cleanup();
}
window.removeEventListener('resize', this.resizeHandler);

This pattern is applied consistently across renderer, controls, and keyboard handlers.


Result

  • Event listener counts stay constant regardless of navigation
  • Window resize remains responsive
  • Keyboard shortcuts execute once per key press
  • Memory remains stable during long sessions
  • No functional regressions observed

Notes

  • Section components were left unchanged
  • No public APIs were modified
  • Changes are limited strictly to listener lifecycle management
  • Verified using DevTools listener inspection and performance monitoring

@Sanchit2662
Copy link
Author

Hi, @EdwardMoyse
This PR fixes an issue where global event listeners were accumulating on each section navigation due to repeated EventDisplay.init() calls on a singleton service.

Thanks for maintaining Phoenix and for taking the time to review this! 🙏

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