From c15557d270f30c861d192d42db607a81d4076588 Mon Sep 17 00:00:00 2001 From: Weird Constructor Date: Thu, 22 Jul 2021 05:31:54 +0200 Subject: [PATCH] fixed thread safety issues with PatternData --- src/dsp/tracker/mod.rs | 43 +++++++++++++++++++++--------------------- src/matrix.rs | 11 +++++------ src/matrix_repr.rs | 4 ++-- src/nodes/node_conf.rs | 6 ++---- tests/basics.rs | 10 +++++----- 5 files changed, 35 insertions(+), 39 deletions(-) diff --git a/src/dsp/tracker/mod.rs b/src/dsp/tracker/mod.rs index 16828eb..3756e5c 100644 --- a/src/dsp/tracker/mod.rs +++ b/src/dsp/tracker/mod.rs @@ -7,8 +7,7 @@ mod sequencer; use ringbuf::{RingBuffer, Producer, Consumer}; -use std::rc::Rc; -use std::cell::RefCell; +use std::sync::{Arc, Mutex}; pub const MAX_COLS : usize = 6; pub const MAX_PATTERN_LEN : usize = 256; @@ -36,7 +35,7 @@ pub enum PatternUpdateMsg { } pub struct Tracker { - data: Rc>, + data: Arc>, data_prod: Producer, seq: Option, seq_cons: Option>, @@ -69,17 +68,17 @@ impl Tracker { let (prod, con) = rb.split(); Self { - data: Rc::new(RefCell::new(PatternData::new(MAX_PATTERN_LEN))), + data: Arc::new(Mutex::new(PatternData::new(MAX_PATTERN_LEN))), data_prod: prod, seq: Some(PatternSequencer::new(MAX_PATTERN_LEN)), seq_cons: Some(con), } } - pub fn data(&self) -> Rc> { self.data.clone() } + pub fn data(&self) -> Arc> { self.data.clone() } pub fn send_one_update(&mut self) -> bool { - let mut data = self.data.borrow_mut(); + let mut data = self.data.lock().unwrap(); for col in 0..MAX_COLS { if data.col_is_modified_reset(col) { @@ -181,11 +180,11 @@ mod tests { let mut t = Tracker::new(); let mut backend = t.get_backend(); - t.data().borrow_mut().set_rows(16); - t.data().borrow_mut().set_col_step_type(0); - t.data().borrow_mut().set_cell_value(0, 0, 0xFFF); - t.data().borrow_mut().set_cell_value(7, 0, 0x777); - t.data().borrow_mut().set_cell_value(15, 0, 0x000); + t.data().lock().unwrap().set_rows(16); + t.data().lock().unwrap().set_col_step_type(0); + t.data().lock().unwrap().set_cell_value(0, 0, 0xFFF); + t.data().lock().unwrap().set_cell_value(7, 0, 0x777); + t.data().lock().unwrap().set_cell_value(15, 0, 0x000); while t.send_one_update() { } while backend.check_updates() { } @@ -209,11 +208,11 @@ mod tests { let mut t = Tracker::new(); let mut backend = t.get_backend(); - t.data().borrow_mut().set_rows(16); - t.data().borrow_mut().set_col_value_type(0); - t.data().borrow_mut().set_cell_value(0, 0, 0xFFF); - t.data().borrow_mut().set_cell_value(7, 0, 0x777); - t.data().borrow_mut().set_cell_value(15, 0, 0x000); + t.data().lock().unwrap().set_rows(16); + t.data().lock().unwrap().set_col_value_type(0); + t.data().lock().unwrap().set_cell_value(0, 0, 0xFFF); + t.data().lock().unwrap().set_cell_value(7, 0, 0x777); + t.data().lock().unwrap().set_cell_value(15, 0, 0x000); while t.send_one_update() { } while backend.check_updates() { } @@ -236,12 +235,12 @@ mod tests { let mut t = Tracker::new(); let mut backend = t.get_backend(); - t.data().borrow_mut().set_rows(4); - t.data().borrow_mut().set_col_gate_type(0); - t.data().borrow_mut().set_cell_value(0, 0, 0xFF7); - t.data().borrow_mut().clear_cell(1, 0); - t.data().borrow_mut().set_cell_value(2, 0, 0xFF0); - t.data().borrow_mut().set_cell_value(3, 0, 0xFFF); + t.data().lock().unwrap().set_rows(4); + t.data().lock().unwrap().set_col_gate_type(0); + t.data().lock().unwrap().set_cell_value(0, 0, 0xFF7); + t.data().lock().unwrap().clear_cell(1, 0); + t.data().lock().unwrap().set_cell_value(2, 0, 0xFF0); + t.data().lock().unwrap().set_cell_value(3, 0, 0xFFF); while t.send_one_update() { } while backend.check_updates() { } diff --git a/src/matrix.rs b/src/matrix.rs index ce932da..bd8a0d8 100644 --- a/src/matrix.rs +++ b/src/matrix.rs @@ -248,8 +248,7 @@ impl Cell { } } -use std::rc::Rc; -use std::cell::RefCell; +use std::sync::{Arc, Mutex}; /// To report back cycle errors from [Matrix::check] and [Matrix::sync]. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] @@ -370,7 +369,7 @@ impl Matrix { } pub fn get_pattern_data(&self, tracker_id: usize) - -> Option>> + -> Option>> { self.config.get_pattern_data(tracker_id) } @@ -537,8 +536,8 @@ impl Matrix { let mut tracker_id = 0; while let Some(pdata) = self.get_pattern_data(tracker_id) { patterns.push( - if pdata.borrow().is_unset() { None } - else { Some(pdata.borrow().to_repr()) }); + if pdata.lock().unwrap().is_unset() { None } + else { Some(pdata.lock().unwrap().to_repr()) }); tracker_id += 1; } @@ -582,7 +581,7 @@ impl Matrix { for (tracker_id, pat) in repr.patterns.iter().enumerate() { if let Some(pat) = pat { if let Some(pd) = self.get_pattern_data(tracker_id) { - pd.borrow_mut().from_repr(pat); + pd.lock().unwrap().from_repr(pat); } } } diff --git a/src/matrix_repr.rs b/src/matrix_repr.rs index 7459125..2c706a3 100644 --- a/src/matrix_repr.rs +++ b/src/matrix_repr.rs @@ -625,7 +625,7 @@ mod tests { { let pat_ref = matrix.get_pattern_data(0).unwrap(); - let mut pat = pat_ref.borrow_mut(); + let mut pat = pat_ref.lock().unwrap(); for col in 0..MAX_COLS { pat.set_col_note_type(col); @@ -663,7 +663,7 @@ mod tests { assert_eq!(s, orig_serial); let pat_ref = matrix.get_pattern_data(0).unwrap(); - let mut pat = pat_ref.borrow_mut(); + let mut pat = pat_ref.lock().unwrap(); for col in 0..MAX_COLS { assert!(pat.is_col_note(col)); diff --git a/src/nodes/node_conf.rs b/src/nodes/node_conf.rs index 47a0db8..ece322b 100644 --- a/src/nodes/node_conf.rs +++ b/src/nodes/node_conf.rs @@ -21,9 +21,7 @@ use crate::monitor::{ use crate::dsp::tracker::{Tracker, PatternData}; use ringbuf::{RingBuffer, Producer}; -use std::rc::Rc; -use std::cell::RefCell; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; use std::collections::HashMap; use triple_buffer::Output; @@ -662,7 +660,7 @@ impl NodeConfigurator { } pub fn get_pattern_data(&self, tracker_id: usize) - -> Option>> + -> Option>> { if tracker_id >= self.trackers.len() { return None; diff --git a/tests/basics.rs b/tests/basics.rs index 7b941aa..d4fe558 100644 --- a/tests/basics.rs +++ b/tests/basics.rs @@ -752,7 +752,7 @@ fn check_matrix_tseq() { let pat = matrix.get_pattern_data(0).unwrap(); { - let mut pr = pat.borrow_mut(); + let mut pr = pat.lock().unwrap(); pr.set_rows(16); pr.set_cell_value(0, 0, 0xFFF); pr.set_cell_value(15, 0, 0x000); @@ -820,7 +820,7 @@ fn check_matrix_tseq_trig() { let pat = matrix.get_pattern_data(0).unwrap(); { - let mut pr = pat.borrow_mut(); + let mut pr = pat.lock().unwrap(); pr.set_rows(16); pr.set_cell_value(0, 0, 0xFFF); pr.set_cell_value(15, 0, 0x000); @@ -889,7 +889,7 @@ fn check_matrix_tseq_gate() { let pat = matrix.get_pattern_data(0).unwrap(); { - let mut pr = pat.borrow_mut(); + let mut pr = pat.lock().unwrap(); pr.set_rows(16); pr.set_col_gate_type(0); // pulse_width: @@ -966,7 +966,7 @@ fn check_matrix_tseq_2col_gate_bug() { let pat = matrix.get_pattern_data(0).unwrap(); { - let mut pr = pat.borrow_mut(); + let mut pr = pat.lock().unwrap(); pr.set_rows(2); pr.set_col_value_type(0); pr.set_col_gate_type(1); @@ -1129,7 +1129,7 @@ fn check_matrix_tseq_perf() { let pat = matrix.get_pattern_data(0).unwrap(); { - let mut pr = pat.borrow_mut(); + let mut pr = pat.lock().unwrap(); pr.set_rows(16); pr.set_col_note_type(0); pr.set_col_gate_type(1);