BUG-03 — paramMap Subscription Memory Leak on Patient Profile Navigation

Severity: High
Component: Patient Profile
File: src/app/patients/patient-profile/patient-profile.ts
Status: Open

Summary

The PatientProfileComponent subscribes to ActivatedRoute.paramMap in ngOnInit but never stores or unsubscribes the subscription. Every navigation between patient profiles creates a new, leaked subscription — causing accumulating background callbacks, potential duplicate data loads, and growing memory usage over a session.

Description

In ngOnInit, the component subscribes to route parameters to detect when the patient ID changes:

// patient-profile.ts — ngOnInit
ngOnInit(): void {
  this.pageHeaderService.title.set('Patients');
  this.pageHeaderService.subtitle.set('');

  this.route.paramMap.subscribe((params) => {
    const id = params.get('id');
    if (id) {
      this.patientId = parseInt(id, 10);
      this.loadPatientData();
    }
  });
}

The subscription is:

1. Never assigned to a class variable
2. Never unsubscribed in ngOnDestroy()

// patient-profile.ts — ngOnDestroy
ngOnDestroy(): void {
  this.pageHeaderService.title.set('');
  this.pageHeaderService.subtitle.set('');
  // ← no subscription cleanup
}

Angular's ActivatedRoute.paramMap is a long-lived observable that does not complete automatically. When the component is destroyed (user navigates away), the subscription is not cleaned up. On the next visit to a patient profile, Angular creates a new component instance with a new subscription — while the previous subscription continues to exist in memory.

After visiting n patient profiles in a session, there are n active subscriptions on paramMap. When route params change (e.g., navigating from /patients/1 to /patients/2), all n subscriptions fire loadPatientData() — potentially in sequence, loading different patient data into the view multiple times.

Steps to Reproduce

1. Open browser DevTools → Memory tab
2. Navigate to http://localhost:4200/#/patients/1
3. Take a heap snapshot
4. Navigate to #/patients/2, then #/patients/3, then back to #/patients/1
5. Take another heap snapshot
6. Compare — PatientProfileComponent instances and their closures remain referenced in memory

Alternatively:

1. Add a console.log inside the paramMap subscribe callback
2. Navigate between several patient profiles
3. On the 4th or 5th navigation, the console shows the callback firing multiple times per navigation

Result: Multiple stale subscriptions accumulate; loadPatientData() may fire multiple times per navigation.
Expected: Only one active subscription exists at any time; it is cleaned up on ngOnDestroy.

Root Cause

The subscription was created without being stored in a variable, making cleanup impossible. The ngOnDestroy hook was implemented (the class does implement OnDestroy) but only handles pageHeaderService reset — the developer forgot to wire up subscription cleanup.

Impact

• Memory leak: Component closures, including references to patient, labResults, and injected services, are kept alive by the orphaned subscriptions.
• Duplicate data loads: After several navigations, loadPatientData() fires multiple times simultaneously when changing patients, causing visible flicker or incorrect patient data briefly appearing.
• Degraded session performance: In a clinical environment where staff navigate between many patient profiles throughout a shift, memory and callback accumulation degrades app responsiveness over time.

Fix Specification

Option A — Store and unsubscribe manually (simple)

import { Subscription } from 'rxjs';

export class PatientProfileComponent implements OnInit, OnDestroy {
  private paramsSub: Subscription | null = null;

  ngOnInit(): void {
    this.pageHeaderService.title.set('Patients');
    this.pageHeaderService.subtitle.set('');

    this.paramsSub = this.route.paramMap.subscribe((params) => {
      const id = params.get('id');
      if (id) {
        this.patientId = parseInt(id, 10);
        this.loadPatientData();
      }
    });
  }

  ngOnDestroy(): void {
    this.pageHeaderService.title.set('');
    this.pageHeaderService.subtitle.set('');
    this.paramsSub?.unsubscribe();
  }
}

Option B — Use takeUntilDestroyed() (preferred Angular 16+ pattern)

import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
import { DestroyRef, inject } from '@angular/core';

export class PatientProfileComponent implements OnInit {
  private destroyRef = inject(DestroyRef);

  ngOnInit(): void {
    this.route.paramMap
      .pipe(takeUntilDestroyed(this.destroyRef))
      .subscribe((params) => {
        const id = params.get('id');
        if (id) {
          this.patientId = parseInt(id, 10);
          this.loadPatientData();
        }
      });
  }

  // ngOnDestroy no longer needed for subscription cleanup
}

Option B is recommended as it is the idiomatic Angular 16+/17+ pattern and removes the need for manual Subscription management. This codebase uses Angular 21 so takeUntilDestroyed is fully available.

Related Issues

• A similar pattern exists in app.ts (ngOnInit) where router.events.subscribe() is also not stored. While the root App component is never destroyed in normal use, it is still a defect pattern that should be corrected for testability.

Acceptance Criteria

• Navigating between 10+ patient profiles does not increase the number of active paramMap subscriptions
• loadPatientData() fires exactly once per patient ID change
• No PatientProfileComponent instances remain referenced in a heap snapshot after navigation away
• ngOnDestroy (or DestroyRef) correctly cleans up the subscription
• All existing patient profile navigation behaviour is preserved