Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: memory leak in StickyScrollFocus #221622

Merged
merged 2 commits into from
Jul 19, 2024
Merged

fix: memory leak in StickyScrollFocus #221622

merged 2 commits into from
Jul 19, 2024

Conversation

SimonSiefke
Copy link
Contributor

Fixes a memory leak in StickyScrollFocus by disposing the focus and blur event listeners when the StickyScrollFocus is disposed.

Before

When exporting a profile 97 times, the number of blur and focus event listeners also increases by 97:

{
  "eventListenersWithStackTrace": [
    {
      "type": "blur",
      "description": "()=>this.onBlur()",
      "objectId": "-8977587310333954231.4.28516",
      "sourceMaps": [""],
      "stack": [
        "listener (vscode/out/vs/base/browser/ui/tree/abstractTree.js:1459:52)",
        "new StickyScrollFocus (vscode/out/vs/base/browser/ui/tree/abstractTree.js:1460:28)",
        "new StickyScrollWidget (vscode/out/vs/base/browser/ui/tree/abstractTree.js:1295:38)",
        "new StickyScrollController (vscode/out/vs/base/browser/ui/tree/abstractTree.js:1067:43)",
        "new AbstractTree (vscode/out/vs/base/browser/ui/tree/abstractTree.js:2122:47)",
        "new ObjectTree (vscode/out/vs/base/browser/ui/tree/objectTree.js:39:13)",
        "Tree.createTree (vscode/out/vs/base/browser/ui/tree/asyncDataTree.js:379:20)",
        "new AsyncDataTree (vscode/out/vs/base/browser/ui/tree/asyncDataTree.js:357:30)",
        "new WorkbenchAsyncDataTree (vscode/out/vs/platform/list/browser/listService.js:846:13)",
        "new Tree (vscode/out/vs/workbench/browser/parts/views/treeView.js:262:5)",
        "InstantiationService._createInstance (vscode/out/vs/platform/instantiation/common/instantiationService.js:139:28)",
        "InstantiationService.createInstance (vscode/out/vs/platform/instantiation/common/instantiationService.js:114:31)",
        "TreeView.createTree (vscode/out/vs/workbench/browser/parts/views/treeView.js:635:76)",
        "TreeView.activate (vscode/out/vs/workbench/browser/parts/views/treeView.js:1713:22)",
        "TreeView.setVisibility (vscode/out/vs/workbench/browser/parts/views/treeView.js:592:22)",
        "UserDataProfilePreviewViewPane.updateTreeVisibility (vscode/out/vs/workbench/browser/parts/views/treeView.js:203:27)",
        "UniqueContainer.value (vscode/out/vs/workbench/browser/parts/views/treeView.js:166:68)",
        "Emitter._deliver (vscode/out/vs/base/common/event.js:790:26)",
        "Emitter._deliverQueue (vscode/out/vs/base/common/event.js:798:22)",
        "Emitter.fire (vscode/out/vs/base/common/event.js:813:22)",
        "UserDataProfilePreviewViewPane.setVisible (vscode/out/vs/workbench/browser/parts/views/viewPane.js:400:53)",
        "vscode/out/vs/workbench/browser/parts/views/viewPaneContainer.js:611:86",
        "Array.map (<anonymous>)",
        "ViewPaneContainer.setVisible (vscode/out/vs/workbench/browser/parts/views/viewPaneContainer.js:611:69)",
        "PaneContainer.setVisible (vscode/out/vs/workbench/browser/panecomposite.js:70:37)",
        "SidebarPart.showComposite (vscode/out/vs/workbench/browser/parts/compositePart.js:177:23)",
        "SidebarPart.showComposite (vscode/out/vs/workbench/browser/parts/paneCompositePart.js:142:19)",
        "SidebarPart.doOpenComposite (vscode/out/vs/workbench/browser/parts/compositePart.js:92:18)",
        "SidebarPart.openComposite (vscode/out/vs/workbench/browser/parts/compositePart.js:69:25)",
        "SidebarPart.doOpenPaneComposite (vscode/out/vs/workbench/browser/parts/paneCompositePart.js:352:25)",
        "SidebarPart.openPaneComposite (vscode/out/vs/workbench/browser/parts/paneCompositePart.js:332:29)",
        "PaneCompositePartService.openPaneComposite (vscode/out/vs/workbench/browser/parts/paneCompositePartService.js:61:66)",
        "ViewsService.openComposite (vscode/out/vs/workbench/services/views/browser/viewsService.js:189:46)",
        "ViewsService.openView (vscode/out/vs/workbench/services/views/browser/viewsService.js:273:50)",
        "UserDataProfileImportExportService.showProfilePreviewView (vscode/out/vs/workbench/services/userDataProfile/browser/userDataProfileImportExportService.js:1236:44)",
        "UserDataProfileImportExportService.showProfileContents (vscode/out/vs/workbench/services/userDataProfile/browser/userDataProfileImportExportService.js:739:28)",
        "UserDataProfileImportExportService.exportProfile2 (vscode/out/vs/workbench/services/userDataProfile/browser/userDataProfileImportExportService.js:228:25)",
        "ExportProfileAction.run (vscode/out/vs/workbench/contrib/userDataProfile/browser/userDataProfile.js:411:72)",
        "handler (vscode/out/vs/platform/actions/common/actions.js:819:50)",
        "InstantiationService.invokeFunction (vscode/out/vs/platform/instantiation/common/instantiationService.js:99:24)",
        "CommandService._tryExecuteCommand (vscode/out/vs/workbench/services/commands/common/commandService.js:91:59)",
        "CommandService.executeCommand (vscode/out/vs/workbench/services/commands/common/commandService.js:63:33)",
        "Object.accept (vscode/out/vs/platform/quickinput/browser/commandsQuickAccess.js:237:165)",
        "UniqueContainer.value (vscode/out/vs/platform/quickinput/browser/pickerQuickAccess.js:199:26)",
        "Emitter._deliver (vscode/out/vs/base/common/event.js:790:26)",
        "Emitter._deliverQueue (vscode/out/vs/base/common/event.js:798:22)",
        "Emitter.fire (vscode/out/vs/base/common/event.js:813:22)",
        "QuickPick.handleAccept (vscode/out/vs/platform/quickinput/browser/quickInput.js:745:41)",
        "UniqueContainer.value (vscode/out/vs/platform/quickinput/browser/quickInput.js:719:30)",
        "Emitter._deliver (vscode/out/vs/base/common/event.js:790:26)",
        "Emitter.fire (vscode/out/vs/base/common/event.js:809:22)",
        "vscode/out/vs/base/common/event.js:116:91",
        "UniqueContainer.value (vscode/out/vs/base/common/event.js:1046:38)",
        "Emitter._deliver (vscode/out/vs/base/common/event.js:790:26)",
        "Emitter._deliverQueue (vscode/out/vs/base/common/event.js:798:22)",
        "Emitter.fire (vscode/out/vs/base/common/event.js:813:22)",
        "Trait._set (vscode/out/vs/base/browser/ui/tree/abstractTree.js:1711:35)",
        "Trait.set (vscode/out/vs/base/browser/ui/tree/abstractTree.js:1701:18)",
        "TreeNodeList.setSelection (vscode/out/vs/base/browser/ui/tree/abstractTree.js:1934:37)",
        "TreeNodeListMouseController.onViewPointer (vscode/out/vs/base/browser/ui/list/listWidget.js:707:27)",
        "TreeNodeListMouseController.onViewPointer (vscode/out/vs/base/browser/ui/tree/abstractTree.js:1832:23)",
        "UniqueContainer.value (vscode/out/vs/base/common/event.js:136:114)",
        "Emitter._deliver (vscode/out/vs/base/common/event.js:790:26)",
        "Emitter.fire (vscode/out/vs/base/common/event.js:809:22)",
        "UniqueContainer.value (vscode/out/vs/base/common/event.js:116:91)",
        "Emitter._deliver (vscode/out/vs/base/common/event.js:790:26)",
        "Emitter.fire (vscode/out/vs/base/common/event.js:809:22)",
        "HTMLDivElement.fn (vscode/out/vs/base/browser/event.js:21:42)"
      ],
      "count": 97
    },
    {
      "type": "focus",
      "description": "()=>this.onFocus()",
      "objectId": "-8977587310333954231.4.28514",
      "sourceMaps": [""],
      "stack": [
        "listener (vscode/out/vs/base/browser/ui/tree/abstractTree.js:1458:53)",
        "new StickyScrollFocus (vscode/out/vs/base/browser/ui/tree/abstractTree.js:1459:28)",
        "new StickyScrollWidget (vscode/out/vs/base/browser/ui/tree/abstractTree.js:1295:38)",
        "new StickyScrollController (vscode/out/vs/base/browser/ui/tree/abstractTree.js:1067:43)",
        "new AbstractTree (vscode/out/vs/base/browser/ui/tree/abstractTree.js:2122:47)",
        "new ObjectTree (vscode/out/vs/base/browser/ui/tree/objectTree.js:39:13)",
        "Tree.createTree (vscode/out/vs/base/browser/ui/tree/asyncDataTree.js:379:20)",
        "new AsyncDataTree (vscode/out/vs/base/browser/ui/tree/asyncDataTree.js:357:30)",
        "new WorkbenchAsyncDataTree (vscode/out/vs/platform/list/browser/listService.js:846:13)",
        "new Tree (vscode/out/vs/workbench/browser/parts/views/treeView.js:262:5)",
        "InstantiationService._createInstance (vscode/out/vs/platform/instantiation/common/instantiationService.js:139:28)",
        "InstantiationService.createInstance (vscode/out/vs/platform/instantiation/common/instantiationService.js:114:31)",
        "TreeView.createTree (vscode/out/vs/workbench/browser/parts/views/treeView.js:635:76)",
        "TreeView.activate (vscode/out/vs/workbench/browser/parts/views/treeView.js:1713:22)",
        "TreeView.setVisibility (vscode/out/vs/workbench/browser/parts/views/treeView.js:592:22)",
        "UserDataProfilePreviewViewPane.updateTreeVisibility (vscode/out/vs/workbench/browser/parts/views/treeView.js:203:27)",
        "UniqueContainer.value (vscode/out/vs/workbench/browser/parts/views/treeView.js:166:68)",
        "Emitter._deliver (vscode/out/vs/base/common/event.js:790:26)",
        "Emitter._deliverQueue (vscode/out/vs/base/common/event.js:798:22)",
        "Emitter.fire (vscode/out/vs/base/common/event.js:813:22)",
        "UserDataProfilePreviewViewPane.setVisible (vscode/out/vs/workbench/browser/parts/views/viewPane.js:400:53)",
        "vscode/out/vs/workbench/browser/parts/views/viewPaneContainer.js:611:86",
        "Array.map (<anonymous>)",
        "ViewPaneContainer.setVisible (vscode/out/vs/workbench/browser/parts/views/viewPaneContainer.js:611:69)",
        "PaneContainer.setVisible (vscode/out/vs/workbench/browser/panecomposite.js:70:37)",
        "SidebarPart.showComposite (vscode/out/vs/workbench/browser/parts/compositePart.js:177:23)",
        "SidebarPart.showComposite (vscode/out/vs/workbench/browser/parts/paneCompositePart.js:142:19)",
        "SidebarPart.doOpenComposite (vscode/out/vs/workbench/browser/parts/compositePart.js:92:18)",
        "SidebarPart.openComposite (vscode/out/vs/workbench/browser/parts/compositePart.js:69:25)",
        "SidebarPart.doOpenPaneComposite (vscode/out/vs/workbench/browser/parts/paneCompositePart.js:352:25)",
        "SidebarPart.openPaneComposite (vscode/out/vs/workbench/browser/parts/paneCompositePart.js:332:29)",
        "PaneCompositePartService.openPaneComposite (vscode/out/vs/workbench/browser/parts/paneCompositePartService.js:61:66)",
        "ViewsService.openComposite (vscode/out/vs/workbench/services/views/browser/viewsService.js:189:46)",
        "ViewsService.openView (vscode/out/vs/workbench/services/views/browser/viewsService.js:273:50)",
        "UserDataProfileImportExportService.showProfilePreviewView (vscode/out/vs/workbench/services/userDataProfile/browser/userDataProfileImportExportService.js:1236:44)",
        "UserDataProfileImportExportService.showProfileContents (vscode/out/vs/workbench/services/userDataProfile/browser/userDataProfileImportExportService.js:739:28)",
        "UserDataProfileImportExportService.exportProfile2 (vscode/out/vs/workbench/services/userDataProfile/browser/userDataProfileImportExportService.js:228:25)",
        "ExportProfileAction.run (vscode/out/vs/workbench/contrib/userDataProfile/browser/userDataProfile.js:411:72)",
        "handler (vscode/out/vs/platform/actions/common/actions.js:819:50)",
        "InstantiationService.invokeFunction (vscode/out/vs/platform/instantiation/common/instantiationService.js:99:24)",
        "CommandService._tryExecuteCommand (vscode/out/vs/workbench/services/commands/common/commandService.js:91:59)",
        "CommandService.executeCommand (vscode/out/vs/workbench/services/commands/common/commandService.js:63:33)",
        "Object.accept (vscode/out/vs/platform/quickinput/browser/commandsQuickAccess.js:237:165)",
        "UniqueContainer.value (vscode/out/vs/platform/quickinput/browser/pickerQuickAccess.js:199:26)",
        "Emitter._deliver (vscode/out/vs/base/common/event.js:790:26)",
        "Emitter._deliverQueue (vscode/out/vs/base/common/event.js:798:22)",
        "Emitter.fire (vscode/out/vs/base/common/event.js:813:22)",
        "QuickPick.handleAccept (vscode/out/vs/platform/quickinput/browser/quickInput.js:745:41)",
        "UniqueContainer.value (vscode/out/vs/platform/quickinput/browser/quickInput.js:719:30)",
        "Emitter._deliver (vscode/out/vs/base/common/event.js:790:26)",
        "Emitter.fire (vscode/out/vs/base/common/event.js:809:22)",
        "vscode/out/vs/base/common/event.js:116:91",
        "UniqueContainer.value (vscode/out/vs/base/common/event.js:1046:38)",
        "Emitter._deliver (vscode/out/vs/base/common/event.js:790:26)",
        "Emitter._deliverQueue (vscode/out/vs/base/common/event.js:798:22)",
        "Emitter.fire (vscode/out/vs/base/common/event.js:813:22)",
        "Trait._set (vscode/out/vs/base/browser/ui/tree/abstractTree.js:1711:35)",
        "Trait.set (vscode/out/vs/base/browser/ui/tree/abstractTree.js:1701:18)",
        "TreeNodeList.setSelection (vscode/out/vs/base/browser/ui/tree/abstractTree.js:1934:37)",
        "TreeNodeListMouseController.onViewPointer (vscode/out/vs/base/browser/ui/list/listWidget.js:707:27)",
        "TreeNodeListMouseController.onViewPointer (vscode/out/vs/base/browser/ui/tree/abstractTree.js:1832:23)",
        "UniqueContainer.value (vscode/out/vs/base/common/event.js:136:114)",
        "Emitter._deliver (vscode/out/vs/base/common/event.js:790:26)",
        "Emitter.fire (vscode/out/vs/base/common/event.js:809:22)",
        "UniqueContainer.value (vscode/out/vs/base/common/event.js:116:91)",
        "Emitter._deliver (vscode/out/vs/base/common/event.js:790:26)",
        "Emitter.fire (vscode/out/vs/base/common/event.js:809:22)",
        "HTMLDivElement.fn (vscode/out/vs/base/browser/event.js:21:42)"
      ],
      "count": 97
    }
  ]
}

After

When exporting a profile 97 times, no additional event listeners are added:

{
  "eventListenersWithStackTrace": [],
  "isLeak": false
}

@aiday-mar
Copy link
Contributor

Hi @SimonSiefke yes thank you for the valuable PR!

@aiday-mar aiday-mar enabled auto-merge (squash) July 13, 2024 11:02
@VSCodeTriageBot VSCodeTriageBot added this to the July 2024 milestone Jul 19, 2024
@aiday-mar aiday-mar merged commit 6ab04ca into microsoft:main Jul 19, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aeschli aeschli aeschli approved these changes

@aiday-mar aiday-mar aiday-mar approved these changes

Assignees

@aiday-mar aiday-mar

Labels
None yet
Projects
None yet
Milestone
July 2024
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@SimonSiefke @aiday-mar @aeschli @VSCodeTriageBot