all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Shannon Sterz <s.sterz@proxmox.com>
To: pdm-devel@lists.proxmox.com
Subject: [pdm-devel] [PATCH datacenter-manager] ui: avoid running tasks and remotes list loads when logged out
Date: Fri, 14 Nov 2025 14:13:05 +0100	[thread overview]
Message-ID: <20251114131305.210762-1-s.sterz@proxmox.com> (raw)

the logic in the DatacenterManagerApp uses `Timeout`s that call
`send_future` to continuously poll the remotes list and running tasks.
the problem there is that given certain conditions these loads can
happen over and over again, even if the user is logged out. this leads
to 401 status codes being returned over and over again.

the result is a race condition that can appear when a user logs in and
one of those load requests is still in-flight. the user will log in
showing the logged in ui. then one of those requests returns with a
401, which triggers an instant log out again. this is made more likely
to happen by the fact that our api has a 3 second delay on requests
that return a 401. this increases the likelihood of a load request
still being in-flight while a login is happening.

these loads happen even though the `Timeout` should be dropped on log
out for a similar reason:

1. when the `DatacenterManagerApp` is created and *thinks* it's logged
   in, it will spawn two `Timeout`s. each with a closure that will
   eventually call `send_future()` to make a request.

   note that the component *thinks* it is logged in, when a cookie is
   present that looks like it has a valid ticket. however, the
   actually ticket may be expired or otherwise invalid. the ui will
   only figure this out once a 401 is returned from the api, as it has
   no way of properly validating a ticket by itself (nor does it even
   check the timestamp or similar as of now).
2. a logout is triggered. most likely because a request returns a 401
   status code. the `LoginPanel` is shown and the `Timeout` is
   dropped. however, since the `Timeout`s spawn futures via
   `send_future()`, these futures will *not* be aborted.
3. if a future handling a request was created before the `Timeout` was
   dropped, it will eventually still complete. since the remotes list
   and running tasks `Timeout`s, use `send_future()`, the message
   returned from these futures will be passed to the component.
4. when such a message is received by the component, it will
   unconditionally create new `Timeout`s.
5. this leads to an infinite loop that always creates new `Timeout`s
   as an old one completes.

the solution is simple: check if we are logged in before creating the
next iterations of `Timeout`s.

note that other components using a similar mechnism to trigger
continous loads are not affected. almost all other components
(especially ones handling continous loads) are only rendered as
children of DatacenterManagerApp. they get destroyed when a logout
happens and the `LoginPanel` is rendered instead. meaning that even if
a loading future is still present, the message it returns can never be
passed to a component. hence, no infinite loop is possible.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 ui/src/main.rs | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/ui/src/main.rs b/ui/src/main.rs
index f8a44f5..026557d 100644
--- a/ui/src/main.rs
+++ b/ui/src/main.rs
@@ -232,10 +232,12 @@ impl Component for DatacenterManagerApp {
                 true
             }
             Msg::TaskChanged => {
-                let running_tasks = self.running_tasks.clone();
-                self.running_tasks_timeout = Some(Timeout::new(3000, move || {
-                    running_tasks.load();
-                }));
+                if self.login_info.is_some() {
+                    let running_tasks = self.running_tasks.clone();
+                    self.running_tasks_timeout = Some(Timeout::new(3000, move || {
+                        running_tasks.load();
+                    }));
+                }
                 false
             } /*
             Msg::SaveFingerprint(fp) => {
@@ -247,7 +249,13 @@ impl Component for DatacenterManagerApp {
             false
             }
              */
-            Msg::RemoteList(remotes) => self.update_remotes(ctx, remotes),
+            Msg::RemoteList(remotes) => {
+                if self.login_info.is_some() {
+                    return self.update_remotes(ctx, remotes);
+                }
+
+                false
+            }
         }
     }
 
-- 
2.47.3



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel


                 reply	other threads:[~2025-11-14 13:12 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251114131305.210762-1-s.sterz@proxmox.com \
    --to=s.sterz@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal