Skip to content

Commit

Permalink
Fix JS memory leak
Browse files Browse the repository at this point in the history
The memory leak can be reproduced easily by opening two web pages of qbittorrent so that the WebUI pages are updated with full_update = true. If you have a large number of torrents, such as 100 torrents, you can observe a rapid increase in memory usage.

This is caused by the incorrect usage of dispose and empty methods in the js codes and none of them garbage collect the elements. If event listeners are added to the DOM elements, those DOM elements will not be garbage collected at all event if they are not referenced or out of the scope, which will cause memory leaks. If some elements are expected to be removed, the correct way is to use destroy method instead.

https://github.com/mootools/mootools-core/blob/master/Docs/Element/Element.md#element-method-dispose-elementdispose
https://github.com/mootools/mootools-core/blob/master/Docs/Element/Element.md#element-method-empty-elementempty
https://github.com/mootools/mootools-core/blob/master/Docs/Element/Element.md#element-method-destroy-elementdestroy

Closes #19034.
PR #19969.
  • Loading branch information
brvphoenix authored Nov 25, 2023
1 parent 19b88b7 commit 9fde563
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 18 deletions.
8 changes: 3 additions & 5 deletions src/webui/www/private/scripts/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ window.addEvent('load', function() {
const categoryList = $('categoryFilterList');
if (!categoryList)
return;
categoryList.empty();
categoryList.getChildren().each(c => c.destroy());

const create_link = function(hash, text, count) {
let display_name = text;
Expand Down Expand Up @@ -540,8 +540,7 @@ window.addEvent('load', function() {
if (tagFilterList === null)
return;

while (tagFilterList.firstChild !== null)
tagFilterList.removeChild(tagFilterList.firstChild);
tagFilterList.getChildren().each(c => c.destroy());

const createLink = function(hash, text, count) {
const html = '<a href="#" onclick="setTagFilter(' + hash + ');return false;">'
Expand Down Expand Up @@ -594,8 +593,7 @@ window.addEvent('load', function() {
if (trackerFilterList === null)
return;

while (trackerFilterList.firstChild !== null)
trackerFilterList.removeChild(trackerFilterList.firstChild);
trackerFilterList.getChildren().each(c => c.destroy());

const createLink = function(hash, text, count) {
const html = '<a href="#" onclick="setTrackerFilter(' + hash + ');return false;">'
Expand Down
2 changes: 1 addition & 1 deletion src/webui/www/private/scripts/contextmenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ window.qBittorrent.ContextMenu = (function() {

updateCategoriesSubMenu: function(category_list) {
const categoryList = $('contextCategoryList');
categoryList.empty();
categoryList.getChildren().each(c => c.destroy());
categoryList.appendChild(new Element('li', {
html: '<a href="javascript:torrentNewCategoryFN();"><img src="images/list-add.svg" alt="QBT_TR(New...)QBT_TR[CONTEXT=TransferListWidget]"/> QBT_TR(New...)QBT_TR[CONTEXT=TransferListWidget]</a>'
}));
Expand Down
10 changes: 4 additions & 6 deletions src/webui/www/private/scripts/dynamicTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -817,8 +817,7 @@ window.qBittorrent.DynamicTable = (function() {
let rowPos = rows.length;

while ((rowPos < trs.length) && (trs.length > 0)) {
trs[trs.length - 1].dispose();
trs.pop();
trs.pop().destroy();
}
},

Expand All @@ -840,7 +839,7 @@ window.qBittorrent.DynamicTable = (function() {
this.selectedRows.erase(rowId);
const tr = this.getTrByRowId(rowId);
if (tr !== null) {
tr.dispose();
tr.destroy();
this.rows.erase(rowId);
return true;
}
Expand All @@ -852,8 +851,7 @@ window.qBittorrent.DynamicTable = (function() {
this.rows.empty();
const trs = this.tableBody.getElements('tr');
while (trs.length > 0) {
trs[trs.length - 1].dispose();
trs.pop();
trs.pop().destroy();
}
},

Expand Down Expand Up @@ -1562,7 +1560,7 @@ window.qBittorrent.DynamicTable = (function() {

if (!country_code) {
if (td.getChildren('img').length > 0)
td.getChildren('img')[0].dispose();
td.getChildren('img')[0].destroy();
return;
}

Expand Down
3 changes: 1 addition & 2 deletions src/webui/www/private/scripts/prop-webseeds.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ window.qBittorrent.PropWebseeds = (function() {

removeRow: function(url) {
if (this.rows.has(url)) {
const tr = this.rows.get(url);
tr.dispose();
this.rows.get(url).destroy();
this.rows.erase(url);
return true;
}
Expand Down
2 changes: 2 additions & 0 deletions src/webui/www/private/views/log.html
Original file line number Diff line number Diff line change
Expand Up @@ -424,4 +424,6 @@

return exports();
})();

Object.freeze(window.qBittorrent.Log);
</script>
4 changes: 2 additions & 2 deletions src/webui/www/private/views/preferences.html
Original file line number Diff line number Diff line change
Expand Up @@ -1884,7 +1884,7 @@
// Advanced Tab
const updateNetworkInterfaces = function(default_iface, default_iface_name) {
const url = 'api/v2/app/networkInterfaceList';
$('networkInterface').empty();
$('networkInterface').getChildren().each(c => c.destroy());
new Request.JSON({
url: url,
method: 'get',
Expand All @@ -1911,7 +1911,7 @@

const updateInterfaceAddresses = function(iface, default_addr) {
const url = 'api/v2/app/networkInterfaceAddressList';
$('optionalIPAddressToBind').empty();
$('optionalIPAddressToBind').getChildren().each(c => c.destroy());
new Request.JSON({
url: url,
method: 'get',
Expand Down
4 changes: 2 additions & 2 deletions src/webui/www/private/views/rss.html
Original file line number Diff line number Diff line change
Expand Up @@ -402,13 +402,13 @@
});
});

$('rssDetailsView').empty();
$('rssDetailsView').getChildren().each(c => c.destroy());
rssArticleTable.updateTable(false);
};

const showDetails = (feedUid, articleID) => {
markArticleAsRead(pathByFeedId.get(feedUid), articleID);
$('rssDetailsView').empty();
$('rssDetailsView').getChildren().each(c => c.destroy());
let article = feedData[feedUid].filter((article) => article.id === articleID)[0];
if (article) {
$('rssDetailsView').append((() => {
Expand Down

0 comments on commit 9fde563

Please sign in to comment.