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.we...@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(+) diff --git a/src/Utils.js b/src/Utils.js index f55b9a5..8a97487 100644 --- a/src/Utils.js +++ b/src/Utils.js @@ -1451,3 +1451,18 @@ Ext.define('Proxmox.Async', { return new Promise((resolve, _reject) => setTimeout(resolve, millis)); }, }); + +Ext.override(Ext.data.Store, { + // If the store's proxy is changed while it is waiting for an AJAX + // response, `onProxyLoad` will still be called for the outdated response. + // To avoid displaying inconsistent information, only process responses + // belonging to the current proxy. + onProxyLoad: function(operation) { + let me = this; + if (operation.getProxy() === me.getProxy()) { + me.callParent(arguments); + } else { + console.log(`ignored outdated response: ${operation.getRequest().getUrl()}`); + } + }, +}); -- 2.30.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
