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

fixes *mut esp_netif_obj not Sync compiler error #338

Closed
wants to merge 3 commits into from

Conversation

cs-clarence
Copy link

Fixes #56 (again).
Implements a new type wrapper SendSyncPtr for arbitrary pointer types and impls Send and Sync on that wrapper. This is just an assertion for the compiler and assumes that the underlying apis from the sdk are relatively thread safe.

@ivmarkov
Copy link
Collaborator

Fixes #56 (again). Implements a new type wrapper SendSyncPtr for arbitrary pointer types and impls Send and Sync on that wrapper. This is just an assertion for the compiler and assumes that the underlying apis from the sdk are relatively thread safe.

There is no such thing as "relatively" thread safe. Either they are thread safe (i.e. you can call methods on the pointer concurrently, from any thread - so you can unsafely "proclaim" the raw C API as Send + Sync) or not. If it is not, and you still mark it as Send + Sync you'll be ending up with Rust UB = weird crashes/misbehaviors in your program, which is against the whole point of using rust in the first place.

esp_netif_t is not thread-safe, contrary to what the ESP IDF documentation says. Did you read this? It is right there, in #56.

What you are doing with this PR is a bit to blindly apply Send + Sync to a bunch of raw C APIs. This is very dangerous. Each API has to be reviewed in detail as to if it can be proclaimed Send + Sync (i.e. "thread safe").

The event loop might be thread safe. The Netif API, as per above - is just not (data races between reading the Netif state and another thread modifying the Netif state).

I'll address the await + tokio::sync::Mutex topic in the issue, a bit later.

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.

esp_netif_obj isn't thread safe
2 participants