Skip to content

Commit f99d000

Browse files
authored
fix(tab-bar): prevent keyboard controller memory leak on rapid mount/unmount (#30906)
Issue number: resolves internal --------- ## What is the current behavior? When `ion-tab-bar` is rapidly mounted and unmounted, a race condition in connectedCallback can cause the keyboard controller to be created after the component has been disconnected. This results in orphaned event listeners (`keyboardWillShow`, `keyboardWillHide`) on the window object that are never cleaned up, causing a memory leak. ## What is the new behavior? The keyboard controller is now properly destroyed in all scenarios: - If the component is disconnected while createKeyboardController is pending, the promise is tracked and destroyed when it resolves - If a new connectedCallback runs before the previous async completes, the stale controller is destroyed The promise tracking pattern ensures only the most recent async operation assigns its result ## Does this introduce a breaking change? - [ ] Yes - [X] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> Current dev build: ``` 8.7.17-dev.11767895575.16ea7cef ``` I was unable to find a way to create tests that accurately identified if this problem was occurring. Memory leaks are notoriously difficult to created automated tests for. I ultimately removed my previous attempts because I didn't want to give a false sense of security.
1 parent 3b3318d commit f99d000

File tree

2 files changed

+46
-2
lines changed

2 files changed

+46
-2
lines changed

core/src/components/footer/footer.tsx

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export class Footer implements ComponentInterface {
2222
private scrollEl?: HTMLElement;
2323
private contentScrollCallback?: () => void;
2424
private keyboardCtrl: KeyboardController | null = null;
25+
private keyboardCtrlPromise: Promise<KeyboardController> | null = null;
2526

2627
@State() private keyboardVisible = false;
2728

@@ -52,7 +53,7 @@ export class Footer implements ComponentInterface {
5253
}
5354

5455
async connectedCallback() {
55-
this.keyboardCtrl = await createKeyboardController(async (keyboardOpen, waitForResize) => {
56+
const promise = createKeyboardController(async (keyboardOpen, waitForResize) => {
5657
/**
5758
* If the keyboard is hiding, then we need to wait
5859
* for the webview to resize. Otherwise, the footer
@@ -64,11 +65,32 @@ export class Footer implements ComponentInterface {
6465

6566
this.keyboardVisible = keyboardOpen; // trigger re-render by updating state
6667
});
68+
this.keyboardCtrlPromise = promise;
69+
70+
const keyboardCtrl = await promise;
71+
72+
/**
73+
* Only assign if this is still the current promise.
74+
* Otherwise, a new connectedCallback has started or
75+
* disconnectedCallback was called, so destroy this instance.
76+
*/
77+
if (this.keyboardCtrlPromise === promise) {
78+
this.keyboardCtrl = keyboardCtrl;
79+
this.keyboardCtrlPromise = null;
80+
} else {
81+
keyboardCtrl.destroy();
82+
}
6783
}
6884

6985
disconnectedCallback() {
86+
if (this.keyboardCtrlPromise) {
87+
this.keyboardCtrlPromise.then((ctrl) => ctrl.destroy());
88+
this.keyboardCtrlPromise = null;
89+
}
90+
7091
if (this.keyboardCtrl) {
7192
this.keyboardCtrl.destroy();
93+
this.keyboardCtrl = null;
7294
}
7395
}
7496

core/src/components/tab-bar/tab-bar.tsx

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import type { TabBarChangedEventDetail } from './tab-bar-interface';
2222
})
2323
export class TabBar implements ComponentInterface {
2424
private keyboardCtrl: KeyboardController | null = null;
25+
private keyboardCtrlPromise: Promise<KeyboardController> | null = null;
2526
private didLoad = false;
2627

2728
@Element() el!: HTMLElement;
@@ -88,7 +89,7 @@ export class TabBar implements ComponentInterface {
8889
}
8990

9091
async connectedCallback() {
91-
this.keyboardCtrl = await createKeyboardController(async (keyboardOpen, waitForResize) => {
92+
const promise = createKeyboardController(async (keyboardOpen, waitForResize) => {
9293
/**
9394
* If the keyboard is hiding, then we need to wait
9495
* for the webview to resize. Otherwise, the tab bar
@@ -100,11 +101,32 @@ export class TabBar implements ComponentInterface {
100101

101102
this.keyboardVisible = keyboardOpen; // trigger re-render by updating state
102103
});
104+
this.keyboardCtrlPromise = promise;
105+
106+
const keyboardCtrl = await promise;
107+
108+
/**
109+
* Only assign if this is still the current promise.
110+
* Otherwise, a new connectedCallback has started or
111+
* disconnectedCallback was called, so destroy this instance.
112+
*/
113+
if (this.keyboardCtrlPromise === promise) {
114+
this.keyboardCtrl = keyboardCtrl;
115+
this.keyboardCtrlPromise = null;
116+
} else {
117+
keyboardCtrl.destroy();
118+
}
103119
}
104120

105121
disconnectedCallback() {
122+
if (this.keyboardCtrlPromise) {
123+
this.keyboardCtrlPromise.then((ctrl) => ctrl.destroy());
124+
this.keyboardCtrlPromise = null;
125+
}
126+
106127
if (this.keyboardCtrl) {
107128
this.keyboardCtrl.destroy();
129+
this.keyboardCtrl = null;
108130
}
109131
}
110132

0 commit comments

Comments
 (0)