dumbmatter / fakeIndexedDB

A pure JS in-memory implementation of the IndexedDB API
Apache License 2.0
585 stars 70 forks source link

Removing a 'complete' event listener in a handler can cause later transactions not to complete #26

Closed quasicomputational closed 5 years ago

quasicomputational commented 5 years ago

Reproduction:

import indexedDB from "fake-indexeddb";

export const STORE = "store";

export const checkForGarbage = () => {
    const dbReq = indexedDB.open("db", 1);
    dbReq.addEventListener("upgradeneeded", (ev) => {
        ev.target.result.createObjectStore(STORE, { autoIncrement: true });
    });
    dbReq.addEventListener("success", (ev) => {
        const db = ev.target.result
        const tx = db.transaction([STORE], "readwrite");
        const complete = () => {
            tx.removeEventListener('complete', complete);
        };
        tx.addEventListener('complete', complete);
        const store = tx.objectStore(STORE);
        store.get(0).addEventListener("success", () => console.log("Done!"));
    });
};

for (let i = 0; i < 5; i++) {
    checkForGarbage();
}

The expected result is to print "Done!" five times. However, only two are produced. Commenting out the body of complete (i.e., removing the removeEventListener call) produces the desired result.

dumbmatter commented 5 years ago

Want me to make a new release, to get this bug fix out? I figure I might as well do that, unless you were planning some other fixes in the very near future.

quasicomputational commented 5 years ago

Any time would be convenient, thanks. This bug isn't even biting me at the moment, as far as I know, but it will be once idb 4.0.0 comes out and I want to upgrade. I don't have anything more queued up imminently - maybe a look at the new WPT failures at some point and seeing if there are easy wins.

dumbmatter commented 5 years ago

Okay, I just published v2.0.6 :)

Out of curiosity, what about idb 4 would trigger this, something internal to that library?

quasicomputational commented 5 years ago

Yeah, basically doing things different internally - it's being more systematic about how it attaches listeners and cleans them up afterwards now. I looked closely at its code because it was the thing that'd changed to make my tests red, but it's not doing anything weird.