fix race condition for headers when saving a new project

microsoft · Sep 9, 2024 · 5ce3336 · 5ce3336
1 parent 124290f
commit 5ce3336
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 54 deletions.
diff --git a/pxtlib/util.ts b/pxtlib/util.ts
@@ -978,27 +978,51 @@ namespace ts.pxtc.Util {
     }
 
     export class PromiseQueue {
-        promises: pxt.Map<(() => Promise<any>)[]> = {};
+        protected allSettledPromise: DeferredPromise<void>;
+        protected queues: pxt.Map<(() => Promise<any>)[]> = {};
 
-        enqueue<T>(id: string, f: () => Promise<T>): Promise<T> {
+        enqueue<T>(id: string, operation: () => Promise<T>): Promise<T> {
             return new Promise<T>((resolve, reject) => {
-                let arr = this.promises[id]
-                if (!arr) {
-                    arr = this.promises[id] = []
+                let queue = this.queues[id]
+                if (!queue) {
+                    queue = this.queues[id] = []
                 }
-                arr.push(() =>
-                    f()
+                queue.push(() =>
+                    operation()
                         .finally(() => {
-                            arr.shift()
-                            if (arr.length == 0)
-                                delete this.promises[id]
-                            else
-                                arr[0]()
+                            queue.shift();
+                            if (queue.length == 0) {
+                                delete this.queues[id];
+
+                                if (this.allSettledPromise && this.areAllPromisesSettled()) {
+                                    this.allSettledPromise.resolve();
+                                    this.allSettledPromise = undefined;
+                                }
+                            }
+                            else {
+                                queue[0]();
+                            }
                         })
-                        .then(resolve, reject))
-                if (arr.length == 1)
-                    arr[0]()
-            })
+                        .then(resolve, reject)
+                );
+                if (queue.length == 1) {
+                    queue[0]();
+                }
+            });
+        }
+
+        async allSettled(): Promise<void> {
+            if (this.areAllPromisesSettled()) return;
+
+            if (!this.allSettledPromise) {
+                this.allSettledPromise = defer();
+            }
+
+            return this.allSettledPromise.promise;
+        }
+
+        protected areAllPromisesSettled() {
+            return Object.keys(this.queues).length === 0
         }
     }
 

diff --git a/webapp/src/workspace.ts b/webapp/src/workspace.ts
@@ -1677,49 +1677,58 @@ let syncAsyncPromise: Promise<pxt.editor.EditorSyncState>;
 export function syncAsync(): Promise<pxt.editor.EditorSyncState> {
     pxt.debug("workspace: sync")
     if (syncAsyncPromise) return syncAsyncPromise;
-    return syncAsyncPromise = impl.listAsync()
-        .catch((e) => {
-            // There might be a problem with the native databases. Switch to memory for this session so the user can at
-            // least use the editor.
-            return switchToMemoryWorkspace("sync failed")
-                .then(() => impl.listAsync());
-        })
-        .then(headers => {
-            const existing = U.toDictionary(allScripts || [], h => h.header.id)
-            // this is an in-place update the header instances
-            allScripts = headers.map(hd => {
-                let ex = existing[hd.id]
-                if (ex) {
-                    if (JSON.stringify(ex.header) !== JSON.stringify(hd)) {
-                        U.jsonCopyFrom(ex.header, hd)
-                        // force reload
-                        ex.text = undefined
-                        ex.version = undefined
-                        data.invalidateHeader("header", hd);
-                        data.invalidateHeader("text", hd);
-                        data.invalidateHeader("pkg-git-status", hd);
-                        data.invalidate("gh-commits:*"); // invalidate commits just in case
-                    }
-                } else {
-                    ex = {
-                        header: hd,
-                        text: undefined,
-                        version: undefined,
-                    }
-                }
-                return ex;
-            })
-            cloudsync.syncAsync(); // sync in background
-        })
-        .then(() => {
-            refreshHeadersSession();
-            return impl.getSyncState ? impl.getSyncState() : null
-        })
+    return syncAsyncPromise = syncCoreAsync()
         .finally(() => {
             syncAsyncPromise = undefined;
         });
 }
 
+async function syncCoreAsync(): Promise<pxt.editor.EditorSyncState> {
+    let headers: pxt.workspace.Header[];
+
+    await headerQ.allSettled();
+
+    try {
+        headers = await impl.listAsync();
+    }
+    catch (e) {
+        // There might be a problem with the native databases. Switch to memory for this session so the user can at
+        // least use the editor.
+        await switchToMemoryWorkspace("sync failed");
+
+        headers = await impl.listAsync();
+    }
+
+    const existing = U.toDictionary(allScripts || [], h => h.header.id)
+    // this is an in-place update the header instances
+    allScripts = headers.map(hd => {
+        let ex = existing[hd.id]
+        if (ex) {
+            if (JSON.stringify(ex.header) !== JSON.stringify(hd)) {
+                U.jsonCopyFrom(ex.header, hd)
+                // force reload
+                ex.text = undefined
+                ex.version = undefined
+                data.invalidateHeader("header", hd);
+                data.invalidateHeader("text", hd);
+                data.invalidateHeader("pkg-git-status", hd);
+                data.invalidate("gh-commits:*"); // invalidate commits just in case
+            }
+        } else {
+            ex = {
+                header: hd,
+                text: undefined,
+                version: undefined,
+            }
+        }
+        return ex;
+    });
+
+    cloudsync.syncAsync(); // sync in background
+    refreshHeadersSession();
+    return impl.getSyncState ? impl.getSyncState() : null
+}
+
 export function resetAsync() {
     allScripts = []
     return impl.resetAsync()