Multi-threaded access to member functions

help
#1

I am a tad confused about how to share a structure between threads such that one thread can update the structures internal data and the second thread can respond to the updates.

The program is reading large blocks of data from a device driver. The reader thread stores the latest data in one of 2 buffers. The response thread uses the buffer which is not currently being used by the other thread:

struct RawBuffer {
    buffer_a_: Arc<Mutex<Vec<u8>>>,
    buffer_b_: Arc<Mutex<Vec<u8>>>,
    using_buffer_a_: AtomicBool,
}

pub struct Reader {
    device_file_: File,
    buffer_: RawBuffer,
    counter_: usize,
}

impl Reader {

    pub fn read_frame(&mut self) {
        let mut guard = match self.buffer_.using_buffer_a_.load(
            Ordering::Relaxed) {
            true => self.buffer_.buffer_a_.lock().unwrap(),
            false => self.buffer_.buffer_b_.lock().unwrap(),
        };

        match self.device_file_.read(&mut *guard) {
            Ok(_) => { self.counter_ += 1; }
            Err(err) => println!("Failed to read file '{}', error = {:?}",
                DEVICE_FILE, err),
        }
    }
    
    pub fn apply<F>(&self, f: F) where F: Fn(&Vec<u8>) {
        let guard = match self.buffer_.using_buffer_a_.load(
            Ordering::Relaxed) {
            true => self.buffer_.buffer_b_.lock().unwrap(),
            false => self.buffer_.buffer_a_.lock().unwrap(),
        };
        let buf = & *guard;

        f(buf);
    }
}

If the calls to read_frame() and apply() are in the same thread everything works, e.g.:

fn reader_thread() {

   let mut reader = Reader::new();

    for _ in 0..1000 {
        reader.read_frame();
        reader.apply(|buf| { 
            // do stuff with buffer
        });
    }
}

When I try to create the reader in my main function such that the apply() method can be called independently, the code fails to compile because the reader thread needs exclusive mutable access, e.g. reader_thread() becomes:

fn read_frames(reader: Arc<frame_reader::Reader>,
               window: Arv<Window>) {
    let num_frames = 6000;

    for _ in 0..num_frames {
        reader.read_frame();
        window.request_redraw();
    }
}

which generates the following error:

Compiling dummy_frame_writer v0.1.0 (/mnt/data-001/Views/View01/ngh-dummy-driver/utils/dummy_frame_writer)
error[E0596]: cannot borrow data in an Arc as mutable
--> src/main.rs:134:9
|
134 | reader.read_frame();
| ^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable

The main() thread needs to update the display whenever a redraw is requested, ergo it needs non-mutable access to the buffer in the reader object:

fn main() {
    let reader = Arc::new(frame_reader::Reader::new();

    // create window and event loop

    let handle = thread::spawn(move || { reader_thread(reader.clone(), window.clone()); });

    event_loop.run(move |event, _, control_flow| {
          // code removed .... 
            Event::RedrawRequested(_) => {
                reader.apply(|buf| {
                    // display buffer details.
                });
            },
            _ => ()
        }
    });
}

How should I structure the code so that the reader can update each buffer and a second thread can use the updated data? Note, reader itself cannot be placed in a Arc<Mutex> because I need to independently lock each buffer...

So far the only mechanism I can think of is to make RawBuffer independent of Reader but that feels wrong.

#2

Your read_frame takes self as mutable borrow. On the "single-threaded" case, you have your reader mutable, so you can call read_frame on it.

In the "multi-threaded" case, your reader is an Arc<_>, not a mut Reader, so you cannot call anything mutable on it. If you want your read_frame to be callable from multiple threads without additional locking, you basically have to make it take self as a shared borrow. It should be possible here, as whole your state is internally-mutable (you can modify both mutexes and atomic through shared borrows).

1 Like
#3

@hashedone - that clears things up. In the full code my Reader struct has some other members which are only used by the reader thread (e.g. the device_file handle which needs to be mutable in the read_frame() function - ergo placing that in its own Arc<Mutex<File>> should resolve the issue.

#4

If something is used only by the reader thread, then I would suggest reorganizing your structure in a way, that it is not connected to the RawBuffer, so it can be kept only by the reader thread. I would imagine an additional argument in the form of reader_ctx: Option<&mut ReaderContext> (or just &mut ReaderContext if the function is called only by the reader) which handles reader-related context and is passed only by the reader. This would save you constantly locking the needless mutex.

You can also have another type wrapping both RawBuffer and ReaderContext (eg. RawBufferMut) which keeps RawBuffer internally as and Arc<Mutex<_>>, and then it forwards all calls to RawBuffer passing ReaderContext where needed.

My feeling is that it is code smell to:

  1. Lock where you never need to lock
  2. Pass a state which you never use (on the writer's side)
1 Like