Skip to content

Commit

Permalink
WebUI: Fix memory leak in context menus
Browse files Browse the repository at this point in the history
This PR fixes a memory leak in context menus. Previously, for some reason, each menu retained references to its target elements without utilizing them further. Since the targets property was accessible/reachable from the root (window object), these references persisted even after the elements were removed from the DOM, preventing them from being garbage collected.
It's easily reproducible - just add a decent amount of torrents, switch between categories multiple times, then capture heap/detached elements snapshot in the Memory tab (Chrome dev tools). The number of detached elements will continue to increase after each category switch and they won't be cleaned up.
[More context](https://github.com/qbittorrent/qBittorrent/pull/22220/files#r1941137796)

PR #22234.
  • Loading branch information
skomerko authored Feb 8, 2025
1 parent e55b59d commit 9392504
Showing 1 changed file with 8 additions and 8 deletions.
16 changes: 8 additions & 8 deletions src/webui/www/private/scripts/contextmenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@ window.qBittorrent.ContextMenu ??= (() => {

// option diffs menu
this.menu = $(this.options.menu);
this.targets = (this.options.targets.length > 0)
? Array.from(document.querySelectorAll(this.options.targets))
: [];

// fx
this.fx = new Fx.Tween(this.menu, {
Expand Down Expand Up @@ -169,15 +166,18 @@ window.qBittorrent.ContextMenu ??= (() => {
}

addTarget(t) {
if (t.hasEventListeners)
return;

// prevent long press from selecting this text
t.style.userSelect = "none";

this.targets[this.targets.length] = t;
t.hasEventListeners = true;
this.setupEventListeners(t);
}

searchAndAddTargets() {
document.querySelectorAll(this.options.targets).forEach((target) => { this.addTarget(target); });
if (this.options.targets.length > 0)
document.querySelectorAll(this.options.targets).forEach((target) => { this.addTarget(target); });
}

triggerMenu(e, el) {
Expand All @@ -199,8 +199,7 @@ window.qBittorrent.ContextMenu ??= (() => {
// get things started
startListener() {
/* all elements */
for (const el of this.targets)
this.setupEventListeners(el);
this.searchAndAddTargets();

/* menu items */
this.menu.addEventListener("click", (e) => {
Expand All @@ -222,6 +221,7 @@ window.qBittorrent.ContextMenu ??= (() => {
// hide on body click
$(document.body).addEventListener("click", () => {
this.hide();
this.options.element = null;
});
}

Expand Down

0 comments on commit 9392504

Please sign in to comment.