provider.destroy() may cause race condition

[HaNdTriX]

Checklist

• [x] Are you reporting a bug? Use github issues for bug reports and feature requests. For general questions, please use https://discuss.yjs.dev/
• [x] Try to report your issue in the correct repository. Yjs consists of many modules. When in doubt, report it to https://github.com/yjs/yjs/issues/

Describe the bug

Calling indexdbProvider.destroy() before sync-Event still triggers sync (async race condition).

To Reproduce To reproduce the behavior, run the following code:

const ydoc = new Y.Doc();
const indexDBProvider = new IndexeddbPersistence("somedoc", ydoc);
indexDBProvider.on("synced", () => {
  console.log("should not be called");
});
console.log("destroy");
indexDBProvider.destroy();

Expected behavior

Calling indexdbProvider.destroy() before sync event never triggers sync.

Environment Information

• All Envs (Node, Browser)
• "y-indexeddb": "^9.0.8", (all versions afaik)
• "yjs": "^13.5.39"

Additional context

I am trying to create react 18 bindings. When using react 18 in strict mode effects may be called twice. (More Info why react 18 is doing this)

useEffect(() => {
  const ydoc = new Y.Doc();

  new IndexeddbPersistence("docName", ydoc);

  return () => {
    ydoc.destroy();
  };
}, []);

Since the provider->sync is still being triggered, all entries inside the doc will be duplicated.

Under the hood the following happens here:

const ydoc = new Y.Doc();
(new IndexeddbPersistence("somedoc", ydoc)).destroy()
new IndexeddbPersistence("somedoc", ydoc)

Afaik: A similar issue applies to y-webrtc.

Possible Solutions:

    this.whenSynced = this._db.then(db => {
+     if (this._isDestroyed) return this
      this.db = db
      const currState = Y.encodeStateAsUpdate(doc)
      return fetchUpdates(this).then(updatesStore => idb.addAutoKey(updatesStore, currState)).then(() => {
        this.emit('synced', [this])
        this.synced = true
        return this
      })
    })

    ...

  destroy () {
    if (this._storeTimeoutId) {
      clearTimeout(this._storeTimeoutId)
    }
    this.doc.off('update', this._storeUpdate)
    this.doc.off('destroy', this.destroy)
+   this._isDestroyed = true
    return this._db.then(db => {
      db.close()
    })
  }

---

@dmonad I really love you work 🙏! Thank you so much providing such a cool package. If you are willing to change this behaviour I am willing to create a PR.

[HaNdTriX]

Here is a demo with a patched version of y-indexeddb: https://github.com/HaNdTriX/next-p2p-chat

[dmonad]

Hi @HaNdTriX, that makes a lot of sense. Happy to accept a PR!