AlphaX/qortal
1
0
Fork 0
forked from Qortal/qortal
Code Issues Pull Requests Projects Releases Wiki Activity

Implemented suggestions from catbref to avoid potential thread safety issue in peer arrays.

Browse Source
This commit is contained in:
CalDescent 2022-03-11 11:27:13 +00:00
parent 7c47e22000
commit a0fedbd4b0
1 changed files with 32 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
src/main/java/org/qortal/network/Network.java
View File

@ -100,9 +100,23 @@ public class Network {
private long nextDisconnectionCheck = 0L; private long nextDisconnectionCheck = 0L;
private final List<PeerData> allKnownPeers = new ArrayList<>(); private final List<PeerData> allKnownPeers = new ArrayList<>();
private List<Peer> connectedPeers = Collections.unmodifiableList(new ArrayList<>());
private List<Peer> handshakedPeers = Collections.unmodifiableList(new ArrayList<>()); /**
private List<Peer> outboundHandshakedPeers = Collections.unmodifiableList(new ArrayList<>()); * Maintain two lists for each subset of peers:
* - A synchronizedList, to be modified when peers are added/removed
* - An immutable List, which is rebuilt automatically to mirror the synchronized list, and is then served to consumers
* This allows for thread safety without having to synchronize every time a thread requests a peer list
*/
private final List<Peer> connectedPeers = Collections.synchronizedList(new ArrayList<>());
private List<Peer> immutableConnectedPeers = Collections.emptyList(); // always rebuilt from mutable, synced list above
private final List<Peer> handshakedPeers = Collections.synchronizedList(new ArrayList<>());
private List<Peer> immutableHandshakedPeers = Collections.emptyList(); // always rebuilt from mutable, synced list above
private final List<Peer> outboundHandshakedPeers = Collections.synchronizedList(new ArrayList<>());
private List<Peer> immutableOutboundHandshakedPeers = Collections.emptyList(); // always rebuilt from mutable, synced list above
private final List<PeerAddress> selfPeers = new ArrayList<>(); private final List<PeerAddress> selfPeers = new ArrayList<>();
private final ExecuteProduceConsume networkEPC; private final ExecuteProduceConsume networkEPC;
@ -240,23 +254,20 @@ public class Network {
} }
public List<Peer> getConnectedPeers() { public List<Peer> getConnectedPeers() {
return this.connectedPeers; return this.immutableConnectedPeers;
} }
public void addConnectedPeer(Peer peer) { public void addConnectedPeer(Peer peer) {
List<Peer> mutableConnectedPeers = new ArrayList<>(this.connectedPeers); this.connectedPeers.add(peer); // thread safe thanks to synchronized list
mutableConnectedPeers.add(peer); this.immutableConnectedPeers = List.copyOf(this.connectedPeers); // also thread safe thanks to synchronized collection's toArray() being fed to List.of(array)
this.connectedPeers = Collections.unmodifiableList(mutableConnectedPeers);
} }
public void removeConnectedPeer(Peer peer) { public void removeConnectedPeer(Peer peer) {
// Firstly remove from handshaked peers // Firstly remove from handshaked peers
this.removeHandshakedPeer(peer); this.removeHandshakedPeer(peer);
// Now remove from connected peers this.connectedPeers.remove(peer); // thread safe thanks to synchronized list
List<Peer> mutableConnectedPeers = new ArrayList<>(this.connectedPeers); this.immutableConnectedPeers = List.copyOf(this.connectedPeers); // also thread safe thanks to synchronized collection's toArray() being fed to List.of(array)
mutableConnectedPeers.remove(peer);
this.connectedPeers = Collections.unmodifiableList(mutableConnectedPeers);
} }
public List<PeerAddress> getSelfPeers() { public List<PeerAddress> getSelfPeers() {
@ -343,13 +354,12 @@ public class Network {
* Returns list of connected peers that have completed handshaking. * Returns list of connected peers that have completed handshaking.
*/ */
public List<Peer> getHandshakedPeers() { public List<Peer> getHandshakedPeers() {
return this.handshakedPeers; return this.immutableHandshakedPeers;
} }
public void addHandshakedPeer(Peer peer) { public void addHandshakedPeer(Peer peer) {
List<Peer> mutableHandshakedPeers = new ArrayList<>(this.handshakedPeers); this.handshakedPeers.add(peer); // thread safe thanks to synchronized list
mutableHandshakedPeers.add(peer); this.immutableHandshakedPeers = List.copyOf(this.handshakedPeers); // also thread safe thanks to synchronized collection's toArray() being fed to List.of(array)
this.handshakedPeers = Collections.unmodifiableList(mutableHandshakedPeers);
// Also add to outbound handshaked peers cache // Also add to outbound handshaked peers cache
if (peer.isOutbound()) { if (peer.isOutbound()) {
@ -358,9 +368,8 @@ public class Network {
} }
public void removeHandshakedPeer(Peer peer) { public void removeHandshakedPeer(Peer peer) {
List<Peer> mutableHandshakedPeers = new ArrayList<>(this.handshakedPeers); this.handshakedPeers.remove(peer); // thread safe thanks to synchronized list
mutableHandshakedPeers.remove(peer); this.immutableHandshakedPeers = List.copyOf(this.handshakedPeers); // also thread safe thanks to synchronized collection's toArray() being fed to List.of(array)
this.handshakedPeers = Collections.unmodifiableList(mutableHandshakedPeers);
// Also remove from outbound handshaked peers cache // Also remove from outbound handshaked peers cache
if (peer.isOutbound()) { if (peer.isOutbound()) {
@ -372,26 +381,23 @@ public class Network {
* Returns list of peers we connected to that have completed handshaking. * Returns list of peers we connected to that have completed handshaking.
*/ */
public List<Peer> getOutboundHandshakedPeers() { public List<Peer> getOutboundHandshakedPeers() {
return this.outboundHandshakedPeers; return this.immutableOutboundHandshakedPeers;
} }
public void addOutboundHandshakedPeer(Peer peer) { public void addOutboundHandshakedPeer(Peer peer) {
if (!peer.isOutbound()) { if (!peer.isOutbound()) {
return; return;
} }
this.outboundHandshakedPeers.add(peer); // thread safe thanks to synchronized list
List<Peer> mutableOutboundHandshakedPeers = new ArrayList<>(this.outboundHandshakedPeers); this.immutableOutboundHandshakedPeers = List.copyOf(this.outboundHandshakedPeers); // also thread safe thanks to synchronized collection's toArray() being fed to List.of(array)
mutableOutboundHandshakedPeers.add(peer);
this.outboundHandshakedPeers = Collections.unmodifiableList(mutableOutboundHandshakedPeers);
} }
public void removeOutboundHandshakedPeer(Peer peer) { public void removeOutboundHandshakedPeer(Peer peer) {
if (!peer.isOutbound()) { if (!peer.isOutbound()) {
return; return;
} }
List<Peer> mutableOutboundHandshakedPeers = new ArrayList<>(this.outboundHandshakedPeers); this.outboundHandshakedPeers.remove(peer); // thread safe thanks to synchronized list
mutableOutboundHandshakedPeers.remove(peer); this.immutableOutboundHandshakedPeers = List.copyOf(this.outboundHandshakedPeers); // also thread safe thanks to synchronized collection's toArray() being fed to List.of(array)
this.outboundHandshakedPeers = Collections.unmodifiableList(mutableOutboundHandshakedPeers);
} }
/** /**