all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Friedrich Weber <f.weber@proxmox.com>
Subject: [pve-devel] applied: [PATCH widget-toolkit] fix #4421: ui: guard setProxy against races of slow vs fast requests
Date: Wed, 8 Mar 2023 07:42:31 +0100	[thread overview]
Message-ID: <87702e73-982b-9702-4fd9-362af3d2c8e4@proxmox.com> (raw)
In-Reply-To: <20230306140314.1150179-1-f.weber@proxmox.com>

Am 06/03/2023 um 15:03 schrieb Friedrich Weber:
> Some UI components use `Ext.data.Store.setProxy` to change their
> associated API endpoint URL in reaction to user input. One example is
> `BackupView`, which calls `setProxy` when the user switches from
> listing backups on storage A to listing backups on storage B. However,
> if A is slow, the UI may receive the response for A *after* the
> response for B. It will then display the contents of A as if they were
> the contents of B, resulting in a UI inconsistency.
> 
> The reason is that `Ext.data.Store` still processes the slow response
> for A, even though it is obsolete. This patch overrides the
> responsible callback of `Ext.data.Store` to only process responses
> belonging to the currently active proxy object. This should rule out
> similar race conditions in all components that use the `setProxy` API.
> In the above example, the patch results in the response for A being
> ignored.
> 
> Ignored responses are logged to the browser console.
> 
> Note that this patch only concerns components that use `setProxy` for
> changing API endpoints. Other components (e.g. those using
> `proxy.setURL` for the same purpose) may be open to similar race
> conditions.
> 
> Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
> ---
>  The original report only concerns the backup view [1], where the race
>  condition is easy to trigger. While ruling out this particular race is
>  simple, I thought it would be worthwhile to rule out race condition of
>  this category for all components. Hence this patch. However, most of the
>  other races are much harder to trigger, so it may be questionable
>  whether a general fix is needed. So if wanted, I can alternatively
>  submit a patch that only fixes the backup view.
> 
>  Also, there are several occurrences of the `proxy.setURL` or `proxy.url = ...`
>  patterns (see [1]) which are also susceptible to race conditions, and which
>  are not fixed by this patch. However, for those, I have not found a nice
>  solution that does not involve changing a lot of call sites. If wanted, I can
>  give it another try, or alternatively only submit patches for components for
>  which triggering the race conditions seems realistic.
> 
>  [1] https://bugzilla.proxmox.com/show_bug.cgi?id=4421
> 
>  src/Utils.js | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
>

applied, thanks!




      parent reply	other threads:[~2023-03-08  6:42 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-06 14:03 [pve-devel] " Friedrich Weber
2023-03-07 18:49 ` Thomas Lamprecht
2023-03-08  6:34   ` Dominik Csapak
2023-03-08  6:42 ` Thomas Lamprecht [this message]

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=87702e73-982b-9702-4fd9-362af3d2c8e4@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=f.weber@proxmox.com \
    --cc=pve-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