rust-lang / rust-lang/rust

Suggest use self instead of &self when involving threads safely errors

Open
#118,096 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

A-diagnostics C-discussion T-compiler
Dominant language
Rust
Stars
119k
Forks
16.2k
PR merge metrics
PR metrics pending

Description

Code
use std::collections::VecDeque;
use std::sync::mpsc::Receiver;
use std::sync::Arc;
use std::sync::Mutex;
use std::thread;
use std::thread::JoinHandle;
pub trait Migration: Send + Sync {}

enum Command {
    Start,
    //Stop,
}

type MigrationTasks = VecDeque<(String, Arc<dyn Migration>)>;
struct MigrationWorker {
    tasks: Arc<Mutex<MigrationTasks>>,
    inbox: Receiver<Command>,
}

impl MigrationWorker {
    pub fn new(tasks: Arc<Mutex<MigrationTasks>>, inbox: Receiver<Command>) -> Self {
        Self { tasks, inbox }
    }

    pub fn start(&self) -> JoinHandle<()> {
        thread::spawn(move || {
            let msg = match self.inbox.recv() {
                Ok(msg) => Some(msg),
                Err(_err) => return,
            };

            if let Some(Command::Start) = msg {
                while let Some((name, _task)) = self.tasks.lock().unwrap().pop_front() {
                    eprintln!("start to run migrate in background: {}", name);
                }
            }
        })
    }
}

fn main() {
    let (tx, rx) = std::sync::mpsc::channel();
    let worker = MigrationWorker::new(Arc::new(Mutex::new(VecDeque::new())), rx);
    tx.send(Command::Start).unwrap();
    worker.start();
}
Current output
error[E0277]: `std::sync::mpsc::Receiver<Command>` cannot be shared between threads safely
   --> src/main.rs:26:23
    |
26  |           thread::spawn(move || {
    |  _________-------------_^
    | |         |
    | |         required by a bound introduced by this call
27  | |             let msg = match self.inbox.recv() {
28  | |                 Ok(msg) => Some(msg),
29  | |                 Err(_err) => return,
...   |
36  | |             }
37  | |         })
    | |_________^ `std::sync::mpsc::Receiver<Command>` cannot be shared between threads safely
    |
    = help: within `MigrationWorker`, the trait `Sync` is not implemented for `std::sync::mpsc::Receiver<Command>`
note: required because it appears within the type `MigrationWorker`
   --> src/main.rs:15:8
    |
15  | struct MigrationWorker {
    |        ^^^^^^^^^^^^^^^
    = note: required for `&MigrationWorker` to implement `Send`
note: required because it's used within this closure
   --> src/main.rs:26:23
    |
26  |         thread::spawn(move || {
    |                       ^^^^^^^
note: required by a bound in `spawn`
   --> /Users/yukang/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/mod.rs:683:8
    |
680 | pub fn spawn<F, T>(f: F) -> JoinHandle<T>
    |        ----- required by a bound in this function
...
683 |     F: Send + 'static,
    |        ^^^^ required by this bound in `spawn`

For more information about this error, try `rustc --explain E0277`.
Desired output
Suggest changing `&self` to `self`

    pub fn start(self) -> JoinHandle<()> {
Rationale and extra context

This may be a relatively easy mistake to make, and the current error message does not seem to help beginners solve the problem very well.

Other cases

No response

Anything else?

When there is more code in fn start(e.g, we change self to &self in this line:
https://github.com/nervosnetwork/ckb/blob/c77a927c1c44b5969b722c731f774c22ab194ff2/db-migration/src/lib.rs#L55), there may be this error messages, again, it's better if we could suggesting how to fix the code.

error[E0521]: borrowed data escapes outside of method
  --> db-migration/src/lib.rs:56:9
   |
55 |       pub fn start(&self) -> JoinHandle<()> {
   |                    -----
   |                    |
   |                    `self` is a reference that is only valid in the method body
   |                    let's call the lifetime of this reference `'1`
56 | /         thread::spawn(move || {
57 | |             let msg = match self.inbox.recv() {
58 | |                 Ok(msg) => Some(msg),
59 | |                 Err(_err) => return,
...  |
84 | |             }
85 | |         })
   | |          ^
   | |          |
   | |__________`self` escapes the method body here
   |            argument requires that `'1` must outlive `'static`

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reproducing the supplied MigrationWorker example and inspecting the E0277 and E0521 diagnostics, including the referenced std:🧵:spawn signature. Compare the current output with the requested suggestion to change &self to self, and verify the behavior against the additional db-migration example. Done means the relevant diagnostics provide an appropriate, tested suggestion without misleading cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.