bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2026-02-04 02:33:33 +00:00
Code Issues Releases Wiki Activity

fix(dirt): replace manual takeUntil with takeUntilDestroyed in AllActivitiesService

Browse Source 
Fixes critical memory leak by replacing manual subscription cleanup
with Angular's automatic DestroyRef-based cleanup pattern.

**Changes:**
- Replace `takeUntil(this._destroy$)` with `takeUntilDestroyed()` for all 3 subscriptions
- Remove unused `_destroy$` Subject and manual `destroy()` method
- Update imports to use `@angular/core/rxjs-interop`

**Why:**
- Manual `destroy()` method was never called anywhere in codebase
- Subscriptions accumulated without cleanup, causing memory leaks
- `takeUntilDestroyed()` uses Angular's DestroyRef for automatic cleanup
- Aligns with ADR-0003 and .claude/CLAUDE.md requirements

**Impact:**
- Automatic subscription cleanup when service context is destroyed
- Prevents memory leaks during hot module reloads and route changes
- Reduces code complexity (no manual lifecycle management needed)

Related to PM-27284
This commit is contained in:
Alex
2025-10-29 13:02:46 -04:00
parent 7a2940673b
commit b1da2cb3d2
1 changed files with 5 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/view/all-activities.service.ts
View File

@@ -1,5 +1,5 @@
import { BehaviorSubject, Subject } from "rxjs";
import { takeUntil } from "rxjs/operators";
import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
import { BehaviorSubject } from "rxjs";
import { ApplicationHealthReportDetailEnriched } from "../../models";
import { OrganizationReportSummary } from "../../models/report-models";
@@ -12,8 +12,6 @@ export class AllActivitiesService {
/// Going forward, this class can be simplified by using the RiskInsightsDataService
/// as it contains the application summary data.
private _destroy$ = new Subject<void>();
private reportSummarySubject$ = new BehaviorSubject<OrganizationReportSummary>({
totalMemberCount: 0,
totalCriticalMemberCount: 0,
@@ -43,7 +41,7 @@ export class AllActivitiesService {
constructor(private dataService: RiskInsightsDataService) {
// All application summary changes
this.dataService.enrichedReportData$.pipe(takeUntil(this._destroy$)).subscribe((report) => {
this.dataService.enrichedReportData$.pipe(takeUntilDestroyed()).subscribe((report) => {
if (report) {
this.setAllAppsReportSummary(report.summaryData);
this.setAllAppsReportDetails(report.reportData);
@@ -51,14 +49,14 @@ export class AllActivitiesService {
});
// Critical application summary changes
this.dataService.criticalReportResults$.pipe(takeUntil(this._destroy$)).subscribe((report) => {
this.dataService.criticalReportResults$.pipe(takeUntilDestroyed()).subscribe((report) => {
if (report) {
this.setCriticalAppsReportSummary(report.summaryData);
}
});
// New applications changes (from orchestrator's reactive pipeline)
this.dataService.newApplications$.pipe(takeUntil(this._destroy$)).subscribe((newApps) => {
this.dataService.newApplications$.pipe(takeUntilDestroyed()).subscribe((newApps) => {
this.setNewApplications(newApps);
});
}
@@ -111,14 +109,4 @@ export class AllActivitiesService {
newApplications: newApps,
});
}
/**
* Cleanup method to prevent memory leaks.
* Should be called when the service is no longer needed.
* Complies with ADR-0003 (Observable Data Services) requirements.
*/
destroy(): void {
this._destroy$.next();
this._destroy$.complete();
}
}