idb correctness currently relies on invalid assumptions about the executor

[Ekleog]

Currently, idb relies on behavior of the futures executor that is not actually specified: it assumes that waking a task from an IndexedDB callback, and then returning from the IndexedDB callback, will result in the awoken task being executed before returning to the browser event loop.

This is not necessarily true, and in particular breaks with the multi-threaded wasm-bindgen executor. I opened an issue upstream, but unfortunately it seems hard to fix, and was closed as wontfix, because that specific behavior of the singlethread executor was never documented in the first place:

• https://github.com/rustwasm/wasm-bindgen/issues/3798

I'm opening this issue to let you know about this current limitation of idb. Please don't just comment on the upstream issue without carefully considering whether you're bringing something new to the table, as that would only be bad vibes for the wasm-bindgen maintainers, and they're doing an awesome job.

On my end I'm planning to fix this in my IndexedDB crate via this change that makes the transaction only ever execute from the callbacks. I verified and it works with the multi-threaded executor, but it also requires a slightly less convenient API. I'm hoping one of the places where I'm opening this issue will have an idea for how to handle this differently :)

Hope that helps!

[devashishdxt]

Thanks for creating this issue. I haven't delved deeper into wasm-bindgen code, but, I tried reproducing this issue using the build flags mentioned here. Even after using these build flags (which should enable multithreaded executor), I was not able to reproduce this issue. Can you please help me finding a way to reproduce this issue?

[Ekleog]

Right, that's because all of idb's current tests run in node, which AFAICT never can use the multi-threaded executor because they don't have web workers.

The exact commit I'm using to reproduce is this one. You can ignore the .envrc and nix-related changes, as they're just a way for me to 100% specify which exact versions of each development tool I'm using, but you'll need at least chromedriver or geckodriver installed. The only meaningful change is configuration of the +atomics feature, and addition of this line:

wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser);

My process to reproduce is 1. run nix-shell (again, you can ignore if you already have the dependencies installed), and 2. inside that shell, run:

cargo test -p idb --test transaction test_transaction_commit

If you remove the +atomics, you should be able to see the test work again, showcasing that idb does work on the singlethread executor in a browser.

[devashishdxt]

idb's current tests run in node, which AFAICT never can use the multi-threaded executor because they don't have web workers.

That is incorrect, idb's tests run in chrome as well as firefox. I just realised that I'm using stable rust and these tests only fail when using nightly (because atomics feature is still in nightly).

[Ekleog]

Hmm that's weird, I was unable to reproduce without adding the run_in_browser line… :shrug:

Happy that you managed to reproduce anyway!

[devashishdxt]

We only need to add run_in_browser once which is present here: https://github.com/devashishdxt/idb/blob/main/idb/tests/web.rs

[LucaCappelletti94]

Any updates on this? Was this problem resolved?

[devashishdxt]

Any updates on this? Was this problem resolved?

Unfortunately there’s no easy way to solve this as IndexedDB has weird auto-commit behaviour. In general, it is a good practice to not have long-lived transactions in IndexedDB. One thing which you can do is to not call .await on individual requests and only call .await on the transaction (this is how put_all works in rexie).

This issue is only for multi-threaded executors in nightly rust when atomics feature is enabled. But, it can also happen in single threaded executors if your code returns the control to browser’s event loop (like making a fetch call).