public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pdm-devel] [PATCH datacenter-manager] ui: avoid running tasks and remotes list loads when logged out
@ 2025-11-14 13:13 Shannon Sterz
  2025-11-19 20:39 ` [pdm-devel] applied: " Thomas Lamprecht
  0 siblings, 1 reply; 2+ messages in thread
From: Shannon Sterz @ 2025-11-14 13:13 UTC (permalink / raw)
  To: pdm-devel

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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* [pdm-devel] applied: [PATCH datacenter-manager] ui: avoid running tasks and remotes list loads when logged out
  2025-11-14 13:13 [pdm-devel] [PATCH datacenter-manager] ui: avoid running tasks and remotes list loads when logged out Shannon Sterz
@ 2025-11-19 20:39 ` Thomas Lamprecht
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Lamprecht @ 2025-11-19 20:39 UTC (permalink / raw)
  To: pdm-devel, Shannon Sterz

On Fri, 14 Nov 2025 14:13:05 +0100, Shannon Sterz wrote:
> 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.
> 
> [...]

Applied, thanks!

[1/1] ui: avoid running tasks and remotes list loads when logged out
      commit: 6a14f4571b0a8cf67949a13391671bba7c91c95b


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


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-11-19 20:40 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-14 13:13 [pdm-devel] [PATCH datacenter-manager] ui: avoid running tasks and remotes list loads when logged out Shannon Sterz
2025-11-19 20:39 ` [pdm-devel] applied: " Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal