[PM-38024] fix: Retain cycle in DefaultTOTPExpirationManager

[amanjeetsingh150]

🎟️ Tracking

Closes #2698

📔 Objective

DefaultTOTPExpirationManager.init schedules a repeating Timer whose block captures self strongly. The cycle (Manager → updateTimer → Timer → block → self) prevents deinit from running, which means cleanup() (the method that invalidates the Timer) is never called. Every instance ever constructed stays alive for the process lifetime.

VaultGroupProcessor.deinit already calls cleanup() on groupTotpExpirationManager as a workaround for this exact issue, suggesting prior awareness. The workaround is missing for searchTotpExpirationManager, and stops the timer without releasing the manager instance itself — symptom, not cause.

Full bug analysis, impact, and rationale for choosing this fix over a structural refactor are in #2698.

Fix

Add [weak self] to the Timer block:

- block: { _ in
-     self.checkForExpirations()
- }
+ block: { [weak self] _ in
+     self?.checkForExpirations()
+ }

Non-breaking, no caller migration. With the cycle broken, deinit runs normally and cleanup() invalidates the Timer automatically. The existing VaultGroupProcessor workaround becomes defensive rather than load-bearing.

Regression test

Added TOTPExpirationManagerLeakTests.test_deallocatesAfterCallerReleasesReference. Uses a weak reference to assert the manager deallocates after its only strong reference is dropped — leaks(1)-based audits don't catch this class of bug because the Timer is held by the main RunLoop, keeping the cycle reachable from a process root. The weak-reference approach catches it cleanly.

Test fails on main with XCTAssertNil failed: "BitwardenShared.DefaultTOTPExpirationManager"; passes with the fix.

📸 Screenshots

n/a — no UI changes.

[CLAassistant]

All committers have signed the CLA.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[bitwarden-bot]

Thank you for your contribution! We've added this to our internal tracking system for review.
ID: PM-38024
Link: https://bitwarden.atlassian.net/browse/PM-38024

Details on our contribution process can be found here: https://contributing.bitwarden.com/contributing/pull-requests/community-pr-process.

[KatherineInCode]

Thank you for your contribution! This is a nice thing to catch!

In addition to my inline comments, I have two more general ones:

• Something went wrong with the CLA bot; can you please re-check and make sure that you've signed it and all that is squared away?
• We have two DefaultTOTPExpirationManager classes that duplicate this code, but this fixes the retain cycle in only one of them. Can you please replicate this and/or find a way to consolidate that object into BitwardenKit?

Thanks!

[KatherineInCode]

🎨 We are trying to write all new tests in Swift Testing. Can you please convert this?

[KatherineInCode]

🎨 This documentation comment is verbose, and not in line with our usual test documentation comments. If there's further context necessary, it can be a comment inside the test function, though I also think a simple test documentation comment will suffice for explaining what is being tested.

[KatherineInCode]

🤔 Is this sleep really necessary? If so, does it have to be 0.2 seconds? As well, it appears that elsewhere in our codebase we document the ns->s conversion, can you please match that convention?

[KatherineInCode]

🎨 This feels overly verbose to me; most of the time we don't include a specific failure message, and when we do, it's a terse explanation of what failed