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

Unneeded RwLock #1

Closed
hgzimmerman opened this issue Dec 17, 2019 · 6 comments
Closed

Unneeded RwLock #1

hgzimmerman opened this issue Dec 17, 2019 · 6 comments

Comments

@hgzimmerman
Copy link

Is your feature request related to a problem? Please describe.
I think there is a RwLock that isn't needed here: https://github.com/jetli/rust-yew-realworld-example-app/blob/master/crates/conduit-wasm/src/agent.rs#L23

Describe the solution you'd like
I think you could use a RefCell instead.

Additional context
Because your app is only running in a single "thread", there isn't a reason to use an RwLock.
It appears that you are only using it to enable interior mutability, which can be provided by a RefCell instead.

In fact you don't need to keep the token in memory and instead just get/set the token directly via the bowser's storage API, but I understand that you could want to avoid that in order to improve performance by a small amount.

@jetli
Copy link
Owner

jetli commented Dec 17, 2019

Yep, it's kind of "cache" here, I'll see RefCell later.

@jetli
Copy link
Owner

jetli commented Dec 21, 2019

@hgzimmerman It seems that lazy_static does not support RefCell due to thread safety:

`std::cell::RefCell<std::option::Option<std::string::String>>` cannot be shared between threads safely
the trait `std::marker::Sync` is not implemented for `std::cell::RefCell<std::option::Option<std::string::String>>`
required by `lazy_static::lazy::Lazy`

@hgzimmerman
Copy link
Author

@jetli
Copy link
Owner

jetli commented Dec 24, 2019

@hgzimmerman I tried thread_local!, It works:

thread_local! {
    pub static TOKEN: RefCell<Option<String>> = RefCell::new(None);
}

of course I have to move the initialization of TOKEN to App component's create.

But I'm thinking that yew supports multi-threading & concurrency, it's totally possible that we might need to use TOKEN in web workers someday or other multi-threading senarios, so I think we'd better use lazy_static for now, and leave it as a demonstration for multi-threading support.
What do you think?

@jetli
Copy link
Owner

jetli commented Dec 24, 2019

or a type Reach = Global agent to manage global state like token/current user info etc? a kind of session for current user.
for now, I have to pass around a current_user info from App to sub components via props, a little tedious.
and we can use agent as a event bus to notify changes, like when a user logs out, the page is redirected to login page.

ok, a related issue: yewstack/yew#576

@jetli
Copy link
Owner

jetli commented Jan 6, 2020

Leave it alone for multi-threading demonstration, closing this

@jetli jetli closed this as completed Jan 6, 2020
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

No branches or pull requests

2 participants