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.

Sign up for GitHub

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

remove unused Mutex #5

Merged
merged 6 commits into from
Jun 17, 2020
Merged

remove unused Mutex #5

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8637003
why lock?
rjwalters Jun 17, 2020
f31d65f
Revert "why lock?"
rjwalters Jun 17, 2020
a5f9bc4
add locks on node
rjwalters Jun 17, 2020
8855555
local node isn't used outside thread
rjwalters Jun 17, 2020
1484529
cleanup simulatednode locks
rjwalters Jun 17, 2020
f734845
add mut
rjwalters Jun 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 19 additions & 33 deletions consensus/scp/tests/mock_network/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ impl SimulatedNetwork {
for node_num in 0..num_nodes {
nodes_map
.get_mut(&test_utils::test_node_id(node_num as u32))
.expect("failed to get node from nodes_map")
.expect("could not find node_id in nodes_map")
.send_stop();
}
drop(nodes_map);
Expand All @@ -210,7 +210,7 @@ impl SimulatedNetwork {
fn push_value(&self, node_id: &NodeID, value: &str) {
self.nodes_map
.lock()
.expect("lock failed on nodes_map getting node")
.expect("lock failed on nodes_map pushing value")
.get(node_id)
.expect("could not find node_id in nodes_map")
.send_value(value);
Expand All @@ -219,7 +219,7 @@ impl SimulatedNetwork {
fn get_ledger(&self, node_id: &NodeID) -> Vec<Vec<String>> {
self.nodes_shared_data
.get(node_id)
.expect("could not find node_id in nodes_map")
.expect("could not find node_id in nodes_shared_data")
.lock()
.expect("lock failed on shared_data getting ledger")
.ledger
Expand All @@ -229,7 +229,7 @@ impl SimulatedNetwork {
fn get_ledger_size(&self, node_id: &NodeID) -> usize {
self.nodes_shared_data
.get(node_id)
.expect("could not find node_id in nodes_map")
.expect("could not find node_id in nodes_shared_data")
.lock()
.expect("lock failed on shared_data getting ledger size")
.ledger_size()
Expand All @@ -245,7 +245,7 @@ impl SimulatedNetwork {
.lock()
.expect("lock failed on nodes_map in broadcast");

log::trace!(logger, "(broadcast) {}", msg.to_display(),);
log::trace!(logger, "(broadcast) {}", msg.to_display());

let amsg = Arc::new(msg);

Expand Down Expand Up @@ -284,7 +284,6 @@ impl SimulatedNodeSharedData {

// A simulated validator node
struct SimulatedNode {
local_node: Arc<Mutex<Node<String, test_utils::TransactionValidationError>>>,
sender: crossbeam_channel::Sender<SimulatedNodeTaskMessage>,
shared_data: Arc<Mutex<SimulatedNodeSharedData>>,
}
Expand All @@ -299,27 +298,22 @@ impl SimulatedNode {
logger: Logger,
) -> (Self, Option<JoinHandle<()>>) {
let (sender, receiver) = crossbeam_channel::unbounded();
let local_node = Arc::new(Mutex::new(Node::new(

let simulated_node = Self {
sender,
shared_data: Arc::new(Mutex::new(SimulatedNodeSharedData { ledger: Vec::new() })),
};

let mut thread_local_node = Node::new(
node_id.clone(),
quorum_set,
test_options.validity_fn.clone(),
test_options.combine_fn.clone(),
logger.clone(),
)));

local_node
.lock()
.expect("lock failed on local node setting scp_timebase_millis")
.scp_timebase = test_options.scp_timebase;

let node = Self {
local_node,
sender,
shared_data: Arc::new(Mutex::new(SimulatedNodeSharedData { ledger: Vec::new() })),
};
);
thread_local_node.scp_timebase = test_options.scp_timebase;

let thread_shared_data = Arc::clone(&node.shared_data);
let thread_local_node = Arc::clone(&node.local_node);
let thread_shared_data = Arc::clone(&simulated_node.shared_data);

// See byzantine_ledger.rs#L626
let max_pending_values_to_nominate: usize = test_options.max_pending_values_to_nominate;
Expand Down Expand Up @@ -387,13 +381,11 @@ impl SimulatedNode {

let outgoing_msg: Option<Msg<String>> = {
thread_local_node
.lock()
.expect("thread_local_node lock failed when nominating value")
.nominate(
current_slot as SlotIndex,
BTreeSet::from_iter(values_to_nominate)
)
.expect("node.nominate() failed")
.expect("nominate() failed")
};

if let Some(outgoing_msg) = outgoing_msg {
Expand All @@ -407,10 +399,8 @@ impl SimulatedNode {
for msg in incoming_msgs.iter() {
let outgoing_msg: Option<Msg<String>> = {
thread_local_node
.lock()
.expect("thread_local_node lock failed when handling msg")
.handle(msg)
.expect("node.handle_msg() failed")
.expect("handle_msg() failed")
};

if let Some(outgoing_msg) = outgoing_msg {
Expand All @@ -422,8 +412,6 @@ impl SimulatedNode {
// Process timeouts (for all slots)
let timeout_msgs: Vec<Msg<String>> = {
thread_local_node
.lock()
.expect("thread_local_node lock failed when processing timeouts")
.process_timeouts()
.into_iter()
.collect()
Expand All @@ -436,9 +424,7 @@ impl SimulatedNode {
// Check if the current slot is done
let new_block:Vec<String> = {
thread_local_node
.lock()
.expect("thread_local_node lock failed when collecting externalized values")
.get_externalized_values(current_slot as SlotIndex)
.get_externalized_values(current_slot as SlotIndex)
};

if !new_block.is_empty() {
Expand Down Expand Up @@ -484,7 +470,7 @@ impl SimulatedNode {
.expect("failed spawning SimulatedNode thread"),
);

(node, thread_handle)
(simulated_node, thread_handle)
}

/// Push value to this node's consensus task.
Expand Down