Steps for committing a transaction don't prevent the cleanup steps from allowing subsequent requests

[Zaggy1024]

I'm working on handling requests serially in Ladybird, since we've had some event ordering issues. In the process, I've come up with a few test cases that are seemingly covered by the spec.

However, I ran into one issue that I can't find an answer for in the spec. Given the following code:

const setupReq = indexedDB.open("commit-prevents-requests", 1);
const storeName = "store";
setupReq.onupgradeneeded = (e) => {
    e.target.result.createObjectStore(storeName);
};
setupReq.onsuccess = (e) => {
    const db = e.target.result;

    const transaction = db.transaction(storeName, "readwrite");
    const store = transaction.objectStore(storeName);

    const firstPut = store.put("foo", "bar");

    transaction.commit();

    firstPut.onsuccess = () => {
        try {
            store.put("lorem", "ipsum");
            console.log("second put did not throw");
        } catch (e) {
            console.log("second put threw " + e.name);
        }
    };
};

I believe the intent is for the second put() to throw an exception, but given the spec steps, it doesn't seem like it would. Specifically, the steps for committing a transaction set the state to committing, but the cleanup Indexed Database transactions steps which run in the microtask checkpoint immediately following the assignment of the onsuccess callback resets the state back to inactive. Is there something else that's intended to prevent this? In our implementation, which tries follows the spec very closely, the second put doesn't trigger an exception, but I may have missed something.

For now, the solution I've found in our implementation (with which it passes all web platform tests and my new tests alike) is to clear the cleanup event loop when committing a transaction, to prevent the state being reset.

[Zaggy1024]

Semi-related, it seems like Firefox, Chromium and Safari all automatically commit a transaction if the cleanup stage is reached and no requests have been made. All I see mentioned in the spec is this:

The implementation must attempt to commit an inactive transaction when all requests placed against the transaction have completed and their returned results handled, no new requests have been placed against the transaction, and the transaction has not been aborted

It seems like this should be an explicit step during cleanup, since that's the only place where it makes sense to detect that it should be committed.

Edit: We also should only commit during the cleanup if the transaction has been scheduled, i.e. isn't blocked by prior readwrite transactions, to ensure proper ordering (covered by WPT IndexedDB/transaction-lifetime-empty.any.html). When a transaction is scheduled, if it has no requests, it should get committed.

[Zaggy1024]

I forgot to note an important part of this issue: The reason why the second put succeeds (in our implementation at least) is because the steps to fire a success event also resets the transaction state to active if it is inactive. Since the cleanup is in a microtask checkpoint, it managed to set the state to inactive before the request could try to fire a success event, so by the time the success fires, the transaction thinks it's ready to go again.

I have a feeling that we should be able to assert that the cleanup steps never act on a transaction that is not in the active state. However, aborting also needs to clear the cleanup event loop for that to be the case (though aborting doesn't cause the state to reset to active in callbacks). I'm trying that setup now in our implementation, and it seems to be passing the WPTs we've imported.