[PATCH] fix #8292: ui: node: StatusView: do not mutate the class-level items array

[PATCH] fix #8292: ui: node: StatusView: do not mutate the class-level items array

[Raphaël Flores]

[-- Attachment #1.1: Type: text/plain, Size: 3254 bytes --]

The repository status widget is appended to `items` in initComponent, but
`items` is declared at class level, so every instance of PVE.node.StatusView
receives the very same array. Each instantiation therefore left one more
config object in that shared array: the k-th display of a node's Summary 
page
constructed k pmxNodeInfoRepoStatus widgets instead of one, and all but 
one of
them were never destroyed (they stay registered in Ext.ComponentManager, 
with
no DOM node, and are not freed by the cycle collector).

The node view is rebuilt on every display of the summary panel, so the waste
grows quadratically with navigation: 322 displays left 52 004 live widget
instances and 208 322 ExtJS components behind, while the DOM stayed flat.

Build a per-instance array instead of pushing onto the shared one.

Signed-off-by: Raphaël Flores <raphael.flores@inrae.fr>
---
  www/manager6/node/StatusView.js | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/manager6/node/StatusView.js 
b/www/manager6/node/StatusView.js
index 0046f5d5..08d68a95 100644
--- a/www/manager6/node/StatusView.js
+++ b/www/manager6/node/StatusView.js
@@ -157,7 +157,7 @@ Ext.define('PVE.node.StatusView', {
              nodetab: 'aptrepositories',
          });

-        me.items.push({
+        me.items = me.items.concat({
              xtype: 'pmxNodeInfoRepoStatus',
              itemId: 'repositoryStatus',
              product: 'Proxmox VE',
-- 
2.56.0

Notes for the reviewers, below the patch trailer on purpose: a `git am`
on this message then leaves the commit message untouched.

The leak surfaced as a tab reaching several GB after a couple of days of 
normal
navigation, and it reproduces in minutes: open a node's Summary page, switch
between it and any other page of the same node, and watch
`Ext.ComponentQuery.query('pmxNodeInfoRepoStatus').length` in the 
browser console.
Reloading the tab clears everything, which is why it goes unnoticed 
until the tab
is old.

Numbers, measured on Proxmox VE 9.2.21 in Firefox 157.0 and Chrome 156 (both
identical, so it is not engine specific): the k-th display of the 
Summary page
adds k repository-status widgets and 4k ExtJS components; 322 displays left
52 004 live widgets, 208 322 components and 2.4 GB of RSS with a flat 
DOM, and
neither a forced GC nor a memory-pressure notification releases any of 
it. A tab
left idle does not grow at all, only the displays count.

Reproducing this one-line change at runtime on the page prototype 
removes the
growth completely: 30 further displays then add 0 widgets and 0 
components, where
without it they added 465 widgets and 1 860 components.

Same user-visible symptom reported on the forum, without a root cause there:
https://forum.proxmox.com/threads/101883

Bugzilla: https://bugzilla.proxmox.com/show_bug.cgi?id=8292

-- 
Raphaël FLORES, INRAE URGI / UR1164, RD10, 78026 Versailles Cedex - France
URGI plateform IT manager / BioinfOmics Research Infrastructure
Forge INRAE project lead / Research engineer
  ** Kindly hosted by ** / CNRGV @INRAE Toulouse
Note: I don't work on Fridays of odd-numbered weeks


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH] fix #8292: ui: node: StatusView: do not mutate the class-level items array

[Dominik Csapak]

Hi,

thanks for your contribution

The patch itself looks good, but did you see
https://pve.proxmox.com/wiki/Developer_Documentation ?

especially the part about signing a CLA?

We'd need that before we apply patches of yours.

best regards
Dominik

On 11/5/26 12:44 PM, Raphaël Flores wrote:
> The repository status widget is appended to `items` in initComponent, but
> `items` is declared at class level, so every instance of 
> PVE.node.StatusView
> receives the very same array. Each instantiation therefore left one more
> config object in that shared array: the k-th display of a node's Summary 
> page
> constructed k pmxNodeInfoRepoStatus widgets instead of one, and all but 
> one of
> them were never destroyed (they stay registered in Ext.ComponentManager, 
> with
> no DOM node, and are not freed by the cycle collector).
> 
> The node view is rebuilt on every display of the summary panel, so the 
> waste
> grows quadratically with navigation: 322 displays left 52 004 live widget
> instances and 208 322 ExtJS components behind, while the DOM stayed flat.
> 
> Build a per-instance array instead of pushing onto the shared one.
> 
> Signed-off-by: Raphaël Flores <raphael.flores@inrae.fr>
> ---
>   www/manager6/node/StatusView.js | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/www/manager6/node/StatusView.js b/www/manager6/node/ 
> StatusView.js
> index 0046f5d5..08d68a95 100644
> --- a/www/manager6/node/StatusView.js
> +++ b/www/manager6/node/StatusView.js
> @@ -157,7 +157,7 @@ Ext.define('PVE.node.StatusView', {
>               nodetab: 'aptrepositories',
>           });
> 
> -        me.items.push({
> +        me.items = me.items.concat({
>               xtype: 'pmxNodeInfoRepoStatus',
>               itemId: 'repositoryStatus',
>               product: 'Proxmox VE',