How to use operator "?" with shared mpsc?

[Wandalen]

I am struggling to apply the knowledge I received in the chapter The ? Operator Can Be Used in Functions That Return Result to my code with shared mpsc channel.

Here is minimal code:

use std::sync::{ mpsc, Arc, Mutex };
use std::error::Error;

fn main() 
{
}

fn sh_rx_get( sh_rx : Arc< Mutex< mpsc::Receiver< usize > > > ) -> Result< usize, Box<dyn Error> >
{
  let rx = sh_rx.lock()?;
  let r = rx.try_recv()?;
  Ok( r )
}

The error I got:

error[E0515]: cannot return value referencing function parameter `sh_rx`
  --> src/main.rs:10:12
   |
10 |   let rx = sh_rx.lock()?;
   |            -----^^^^^^^^
   |            |
   |            returns a value referencing data owned by the current function
   |            `sh_rx` is borrowed here

Online playground.

Any suggestion or hint?

[alice]

It is pretty much never correct to put an mpsc receiver inside an Arc/Mutex. Either pass it by value, or if you need several receivers, use a channel type that supports that (e.g. crossbeam)

[Wandalen]

@alice here is full code:

use std::sync::{ mpsc, Arc, Mutex };
use std::thread;
use std::error::Error;

fn main() 
{
  let ( tx, rx ) = mpsc::sync_channel( 1024 );
  let sh_rx = Arc::new( Mutex::new( rx ) );

  for tid in 1..=4
  {
    thread_performer( tid, sh_rx.clone() );
  }

  thread_producer( tx ).join().unwrap();
}

//

fn prime_is( u : usize ) -> bool
{
  ( 2 .. u ).all( | i | { u % i != 0 } )
}

//

fn thread_producer( tx : mpsc::SyncSender<usize> ) -> thread::JoinHandle<()>
{
  thread::spawn( move ||
  {
    for i in 100_000 .. 150_000
    {
      tx.send( i ).unwrap();
    }
  })
}

//

fn thread_performer( tid : usize, sh_rx : Arc< Mutex< mpsc::Receiver<usize> > > ) 
{
  thread::spawn( move ||
  {
    loop
    {
      let mut n = 0;

      // n = sh_rx.lock()?.try_recv()?;

      match sh_rx.lock()
      {
        Ok( rx ) => 
        {
          match rx.try_recv()
          {
            Ok( _n ) => { n = _n; },
            Err( _ ) => (),
          }
        }
        Err( _ ) => (),
      }

      if n != 0 && prime_is( n )
      {
        println!( "Thread::{} found prime {}", tid, n );
      }
    }
  });
}

fn sh_rx_get( sh_rx : Arc< Mutex< mpsc::Receiver< usize > > > ) -> Result< usize, Box<dyn Error> >
{
  let rx = sh_rx.lock()?;
  let r = rx.try_recv()?;
  Ok( r )
}

Playground

It's inspired by the tutorial.

Any suggestion how to use operator "?" in the code?

[alice]

The lock and try_recv methods are usually not used with the question mark operator, but either an unwrap or match. We should consider what the errors actually mean. The lock method will fail if the mutex is poisoned. This happens if some other thread panics while keeping the mutex locked. Most people unwrap this error instead of using the question-mark operator.

As for try_recv, that method fails if there are not any messages to receive at this time. What should happen in that case? You should also ask yourself if poisoned mutex and empty channel should be handled in the same manner?

Anyway, as I said above, you should not be using an Arc/Mutex here. Use a crossbeam channel instead.

[Wandalen]

The reason for this error is that std::sync::Mutex::lock() returns a Result<MutexGuard<'_, T>, PoisonError<MutexGuard<'_, T>>>. As you can see, the error case of that Result carries a lifetime that borrows the original lock (the lifetime '_ in MutexGuard). The compiler is trying to tell you that returning an error via the ? would return a borrowed value - PoisonError<MutexGuard<'_, T>>> - that was created, and is only valid, in this function; this would create a dangling reference and is therefor a compile error.

The offending local borrow ("data owned by the current function") is created implicitly when sh_rx.lock() is called. Rust will automatically reference the Arc sh_rx and then dereference from Arc to the Mutex, where .lock() is actually defined.

The reason lock() returns a value that borrows the lock even in the error case is that locking fails if another thread panicked while holding the lock; this could mean that the data protected by the lock is in an inconsistent state. PoisonError has an into_inner() method which acquires the lock regardless of this, and it requires the a borrow of the original lock to do so.

You can fix this by converting the PoisonError into some other type which does not borrow the lock. For example:

let rx = sh_rx.lock().map_err(|e| e.to_string())?;

This works because there is a From<String> for Box<dyn Error> in std, which is automatically used by the ?-operator to convert the String into the Box<dyn Error> which your function returns.