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

Incorrect shared mutable state example at website ntex.rs #497

Open
justforcomment opened this issue Jan 14, 2025 · 6 comments
Open

Incorrect shared mutable state example at website ntex.rs #497

justforcomment opened this issue Jan 14, 2025 · 6 comments

Comments

@justforcomment
Copy link

Page contains the next code fragment:
https://ntex.rs/code#shared-mutable-state

`use ntex::web;
use std::sync::Mutex;

struct AppStateWithCounter {
counter: Mutex, // <- Mutex is necessary to mutate safely across threads
}

async fn index(data: web::types::State) -> String {
let mut counter = data.counter.lock().unwrap(); // <- get counter's MutexGuard
*counter += 1; // <- access counter inside MutexGuard

format!("Request number: {counter}") // <- response with count

}

#[ntex::main]
async fn main() -> std::io::Result<()> {
// Note: app state created outside HttpServer::new closure
let counter = AppStateWithCounter {
counter: Mutex::new(0),
};

web::HttpServer::new(move || {
    // move counter into the closure
    web::App::new()
        // **!! Incorrect usage is here !!**
        .state(counter.clone()) // <- register the created data
        .route("/", web::get().to(index))
})
.bind(("127.0.0.1", 8080))?
.run()
.await

}
`

  1. Of course we can't clone an owned type containing a mutex (that not implements Clone)..
    Is mistake? Is this website run by someone else? Moreover, it is not always available (ERR_TIMED_OUT);

  2. The site also recommends not wrapping a shared mutable state in Arc, since this is done internally by the framework. When many examples still wrap the state explicitly in Arc, which seems to lead to duplication -- right?

@fafhrd91
Copy link
Member

@leon3s could you update example in doc. I reviewed example. and yes, we do not support multi threading state, Arc must be used explicitly. also regarding docs hosting, should we push docs to github instead?

@leon3s
Copy link
Member

leon3s commented Jan 14, 2025

I can unlock time this weekend to update the doc, if the op can make a PR for the doc it will be deployed automatically once merged. As for the (ERR_TIMED_OUT) it can come from the client aswell uptime kuma show 100% uptime for me.. but we can see to migrate if you want but i rather not and try to find the issue if there is really one

@leon3s
Copy link
Member

leon3s commented Jan 14, 2025

If you give me a discord url for webhook i can add it to uptime kuma, it will send a message if the service go down, i have it but in my own discord

@fafhrd91
Copy link
Member

@leon3s I gave you admin role on discord server

@leon3s
Copy link
Member

leon3s commented Jan 14, 2025

Alright i'll setup this asap

@leon3s
Copy link
Member

leon3s commented Jan 14, 2025

I have setup the webhook and also added the website on this page: https://status.next-hat.com/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants