fix(Project): prevent potential data corruption in saveToCloud and saveToLocalCache

spx-gui/src/components/project/ProjectCreateModal.vue

@@ -99,7 +99,7 @@ const handleSubmit = useMessageHandle(
     project.addSprite(sprite)
     await sprite.autoFit()
     // upload project content & call API addProject, TODO: maybe this should be extracted to `@/models`?
-    const files = project.export()[1]
+    const [, files] = await project.export()
     const { fileCollection } = await saveFiles(files)
     const projectData = await addProject({
       name: form.value.name,

spx-gui/src/models/animation.test.ts

@@ -85,7 +85,7 @@ describe('Animation', () => {
     const project = makeProject()
     project.sprites[0].animations[0].setSound(project.sounds[0].name)
 
-    const [metadata, files] = project.export()
+    const [metadata, files] = await project.export()
     const delayedFiles: Files = Object.fromEntries(
       Object.entries(files).map(([path, file]) => [path, delayFile(file!, 50)])
     )

spx-gui/src/models/project/history.ts

@@ -5,7 +5,6 @@
 
 import { shallowReactive } from 'vue'
 import type { LocaleMessage } from '@/utils/i18n'
-import Mutex from '@/utils/mutex'
 import type { Files } from '../common/file'
 import type { Project } from '.'
 
@@ -27,8 +26,6 @@ export type State = {
 }
 
 export class History {
-  private mutex = new Mutex()
-
   constructor(
     private project: Project,
     /**
@@ -71,7 +68,7 @@ export class History {
   }
 
   redo() {
-    return this.mutex.runExclusive(() => this.goto(this.index + 1))
+    return this.project.historyMutex.runExclusive(() => this.goto(this.index + 1))
   }
 
   getUndoAction() {
@@ -80,11 +77,11 @@ export class History {
   }
 
   undo() {
-    return this.mutex.runExclusive(() => this.goto(this.index - 1))
+    return this.project.historyMutex.runExclusive(() => this.goto(this.index - 1))
   }
 
   doAction<T>(action: Action, fn: () => T | Promise<T>): Promise<T> {
-    return this.mutex.runExclusive(async () => {
+    return this.project.historyMutex.runExclusive(async () => {
       // history after current state (for redo) will be discarded on any action
       this.states.splice(this.index)
 

spx-gui/src/models/project/index.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect } from 'vitest'
+import { describe, it, expect, vi } from 'vitest'
 import { Sprite } from '../sprite'
 import { Animation } from '../animation'
 import { Sound } from '../sound'
@@ -124,4 +124,16 @@ describe('Project', () => {
     project.removeSprite('Sprite2')
     expect(project.selected).toBeNull()
   })
+
+  it('should throw an error when saving a disposed project', async () => {
+    const project = makeProject()
+    const saveToLocalCacheMethod = vi.spyOn(project, 'saveToLocalCache' as any)
+
+    project.dispose()
+
+    await expect(project.saveToCloud()).rejects.toThrow('disposed')
+
+    await expect((project as any).saveToLocalCache('key')).rejects.toThrow('disposed')
+    expect(saveToLocalCacheMethod).toHaveBeenCalledWith('key')
+  })
 })

spx-gui/src/models/project/index.ts

@@ -21,6 +21,7 @@ import { Sprite } from '../sprite'
 import { Sound } from '../sound'
 import type { RawWidgetConfig } from '../widget'
 import { History } from './history'
+import Mutex from '@/utils/mutex'
 
 export type { Action } from './history'
 
@@ -251,6 +252,7 @@ export class Project extends Disposable {
   }
 
   history: History
+  historyMutex = new Mutex()
 
   constructor() {
     super()
@@ -267,10 +269,24 @@ export class Project extends Disposable {
     return reactiveThis
   }
 
-  private applyMetadata(metadata: Metadata) {
+  private loadMetadata(metadata: Metadata) {
     assign<Project>(this, metadata)
   }
 
+  private exportMetadata(): Metadata {
+    return {
+      id: this.id,
+      owner: this.owner,
+      name: this.name,
+      isPublic: this.isPublic,
+      version: this.version,
+      cTime: this.cTime,
+      uTime: this.uTime,
+      filesHash: this.filesHash,
+      lastSyncedFilesHash: this.lastSyncedFilesHash
+    }
+  }
+
   async loadGameFiles(files: Files) {
     const configFile = files[projectConfigFilePath]
     const config: RawProjectConfig = {}
@@ -330,25 +346,13 @@ export class Project extends Disposable {
 
   /** Load with metadata & game files */
   async load(metadata: Metadata, files: Files) {
-    this.applyMetadata(metadata)
+    this.loadMetadata(metadata)
     await this.loadGameFiles(files)
   }
 
   /** Export metadata & game files */
-  export(): [Metadata, Files] {
-    const metadata: Metadata = {
-      id: this.id,
-      owner: this.owner,
-      name: this.name,
-      isPublic: this.isPublic,
-      version: this.version,
-      cTime: this.cTime,
-      uTime: this.uTime,
-      filesHash: this.filesHash,
-      lastSyncedFilesHash: this.lastSyncedFilesHash
-    }
-    const files = this.exportGameFiles()
-    return [metadata, files]
+  async export(): Promise<[Metadata, Files]> {
+    return this.historyMutex.runExclusive(() => [this.exportMetadata(), this.exportGameFiles()])
   }
 
   async loadGbpFile(file: globalThis.File) {
@@ -363,7 +367,7 @@ export class Project extends Disposable {
   }
 
   async exportGbpFile() {
-    const [metadata, files] = this.export()
+    const [metadata, files] = await this.export()
     return await gbpHelper.save(metadata, files)
   }
 
@@ -380,9 +384,10 @@ export class Project extends Disposable {
 
   /** Save to cloud */
   async saveToCloud() {
-    const [metadata, files] = this.export()
+    const [metadata, files] = await this.export()
+    if (this.isDisposed) throw new Error('disposed')
     const saved = await cloudHelper.save(metadata, files)
-    this.applyMetadata(saved.metadata)
+    this.loadMetadata(saved.metadata)
     this.lastSyncedFilesHash = await hashFiles(files)
   }
 
@@ -396,7 +401,8 @@ export class Project extends Disposable {
 
   /** Save to local cache */
   private async saveToLocalCache(key: string) {
-    const [metadata, files] = this.export()
+    const [metadata, files] = await this.export()
+    if (this.isDisposed) throw new Error('disposed')
     await localHelper.save(key, metadata, files)
   }
 
@@ -486,7 +492,11 @@ export class Project extends Disposable {
         else delazyLoadGameFiles()
       }
     })()
-    this.addDisposer(watch(() => this.export(), autoSaveToLocalCache, { immediate: true }))
+    this.addDisposer(
+      watch(() => [this.exportMetadata(), this.exportGameFiles()], autoSaveToLocalCache, {
+        immediate: true
+      })
+    )
 
     // watch for autoSaveMode switch, and trigger auto save accordingly
     this.addDisposer(

spx-gui/src/utils/disposable.ts

@@ -6,15 +6,23 @@
 export type Disposer = () => void
 
 export class Disposable {
-  _disposers: Disposer[] = []
+  private disposers: Disposer[] = []
+
+  private _isDisposed = false
+  get isDisposed() {
+    return this._isDisposed
+  }
 
   addDisposer = (disposer: Disposer) => {
-    this._disposers.push(disposer)
+    if (this._isDisposed) throw new Error('disposed')
+    this.disposers.push(disposer)
   }
 
   dispose = () => {
-    while (this._disposers.length > 0) {
-      this._disposers.pop()?.()
+    if (this._isDisposed) return
+    this._isDisposed = true
+    while (this.disposers.length > 0) {
+      this.disposers.pop()?.()
     }
   }
 }

spx-gui/src/utils/exception.ts

@@ -30,7 +30,7 @@ export class DefaultException extends Exception {
 }
 
 /**
- * Cancelled is a special exception, it stands for a "cancel operation" because of user ineraction.
+ * Cancelled is a special exception, it stands for a "cancel operation" because of user interaction.
  * Like other exceptions, it breaks normal flows, while it is supposed to be ignored by all user-feedback components,
  * so the user will not be notified of cancelled exceptions.
  */