tauri-apps / plugins-workspace

All of the official Tauri plugins in one place!
https://tauri.app
Apache License 2.0
979 stars 274 forks source link

fix(sql) Allow tauri-plugin-sql to work when Tauri is running async #2038

Closed johncarmack1984 closed 1 week ago

johncarmack1984 commented 1 week ago

At present, tauri-plugin-sql only works when the main Tauri thread is using the default, synchronous runtime.

In certain configurations, running Tauri asynchronously can be necessary, ie when using TaurPC or other dependencies that require it.

With the current configuration, running Tauri asynchronously while trying to use tauri-plugin-sql yields this error:

thread 'main' panicked at ~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.40.0/src/runtime/scheduler/multi_thread/mod.rs:86:9:
Cannot start a runtime from within a runtime. This happens because a function (like `block_on`) attempted to block the current thread while the thread is being used to drive asynchronous tasks.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

By changing the block_on function call in sql/lib.rs to the macro provided, the plugin works flawlessly, in both synchronous and asynchronous runtimes.

Please let me know if more information is needed!

FabianLars commented 1 week ago

Makes me wonder why we didn't make https://github.com/tauri-apps/tauri/blob/dev/crates%2Ftauri%2Fsrc%2Fasync_runtime.rs#L288 the default 🤔

FabianLars commented 1 week ago

Actually, I think having a centralized approach would be better, this could happen in every plugin because they kinda as expect a sync main fn

FabianLars commented 1 week ago

@amrbashir do you think pub exposing safe_block_on in core makes sense?

github-actions[bot] commented 1 week ago

Package Changes Through 837f1e93004de08624fe0f51d91fe35976de15bd

There are 10 changes which include upload with minor, upload-js with minor, deep-link with patch, deep-link-js with patch, log-plugin with patch, log-js with patch, fs with patch, fs-js with patch, localhost with minor, sql with patch

Planned Package Versions The following package releases are the planned based on the context of changes in this pull request. | package | current | next | |----|----|----| | api-example | 2.0.5 | 2.0.6 | api-example-js | 2.0.2 | 2.0.3 | deep-link-example-js | 2.0.0 | 2.0.1 | deep-link | 2.0.1 | 2.0.2 | deep-link-js | 2.0.0 | 2.0.1 | fs | 2.0.3 | 2.0.4 | fs-js | 2.0.2 | 2.0.3 | dialog | 2.0.3 | 2.0.4 | http | 2.0.3 | 2.0.4 | localhost | 2.0.1 | 2.1.0 | log-plugin | 2.0.2 | 2.0.3 | log-js | 2.0.0 | 2.0.1 | persisted-scope | 2.0.3 | 2.0.4 | single-instance | 2.0.1 | 2.0.2 | sql | 2.0.2 | 2.0.3 | upload | 2.1.0 | 2.2.0 | upload-js | 2.1.0 | 2.2.0 |

Add another change file through the GitHub UI by following this link.


Read about change files or the docs at github.com/jbolda/covector

amrbashir commented 1 week ago

maybe, idk, I don't use async that much

FabianLars commented 1 week ago

okay no problem, let's keep it plugin-local for now and see if this gets more common.

@johncarmack1984 would you mind checking if the safe_block_on approach also works here? I assume there's a reason it doesn't use block_in_place.

Also, i think a code comment explaining why this was added (so nobody removes it by accident) would be valuable.

And idk if it's just me but i'd rather not solve this with a macro but just a simple function call if there's no good reason for a macro.

Lastly, we'll also need a changefile.

johncarmack1984 commented 1 week ago

Sure thing! If I clone Tauri locally and modify crates/tauri/src/async_runtime.rs to make block_in_place public, then use tauri::async_runtime::safe_block_on in tauri-plugin-sql instead of the macro, I get:

`dyn StdError` cannot be sent between threads safely
the trait `Send` is not implemented for `dyn StdError`, which is required by `Result<(), _>: Send`
required for `Unique<dyn StdError>` to implement `Send`

I'll add an explainer comment and get started on a changefile, hopefully will have it later today!

Will also see if functionality is preserved when using a function vs a macro and report back if not.

johncarmack1984 commented 1 week ago

Success converting macro to function, have added changefile and explainer comment! Please let me know if I can provide anything else! :)

FabianLars commented 1 week ago

Okay now i remember why safe_block_on was bigger than that. Your change currently panics in sync contexts.

johncarmack1984 commented 1 week ago

Oof... yikes, I'll take another crack at it. Any initial thoughts on approaches that might satisfy both? If not I'll troubleshoot it today.

FabianLars commented 1 week ago

a modified version of safe_block_on. Using Handle::try_current to check if there's a runtime and then using block_in_place or block_on directly.

johncarmack1984 commented 1 week ago

a modified version of safe_block_on. Using Handle::try_current to check if there's a runtime and then using block_in_place or block_on directly.

Worked a charm; sync and async both operating locally. Thanks for catching that!

johncarmack1984 commented 1 week ago

(just want to pop in here and give credit to @jodavaho for the initial macro, thanks for the collaboration everybody)