Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Conversation

johncarmack1984
Copy link
Contributor

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!

@johncarmack1984 johncarmack1984 requested a review from a team as a code owner November 12, 2024 16:39
@johncarmack1984 johncarmack1984 changed the title Allow tauri-plugin-sql to work when Tauri is running async fix(sql) Allow tauri-plugin-sql to work when Tauri is running async Nov 12, 2024
@FabianLars
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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

Copy link
Contributor

github-actions bot commented Nov 12, 2024

Package Changes Through 837f1e9

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
Copy link
Member

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

@FabianLars
Copy link
Member

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
Copy link
Contributor Author

johncarmack1984 commented Nov 13, 2024

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
Copy link
Contributor Author

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

@FabianLars
Copy link
Member

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

@johncarmack1984
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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!

@FabianLars FabianLars merged commit 90ef77c into tauri-apps:v2 Nov 13, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants