[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [metrics-lib/release] Make NetworkStatusEntryImpl#parseSLine thread-safe.
commit bde697f44702c3f93cfff7953ff8f5962582e2e1
Author: Karsten Loesing <karsten.loesing@xxxxxxx>
Date: Thu Oct 24 21:29:07 2019 +0200
Make NetworkStatusEntryImpl#parseSLine thread-safe.
The bug was that we accessed static class members, namely the two maps
NetworkStatusEntryImpl#flagIndexes and #flagStrings, during instance
creation without synchronization. This worked just fine with a single
thread creating instances, but it breaks with multiple threads doing
that at the same time.
The fix is to keep a separate map per NetworkStatusImpl instance and
share that between all its NetworkStatusEntryImpl instances. This
doesn't save as much memory as sharing maps between all
NetworksStatusEntryImpl instances ever created, but it's a reasonable
compromise between memory and runtime efficiency. In contrast to that,
synchronizing map access would have put a major runtime performance
penalty on parsing.
Fixes #32194.
---
CHANGELOG.md | 3 +++
.../descriptor/impl/NetworkStatusEntryImpl.java | 27 +++++++++++-----------
.../descriptor/impl/NetworkStatusImpl.java | 8 ++++++-
.../impl/RelayNetworkStatusConsensusImpl.java | 3 ++-
4 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9c7610a..73022cf 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,8 @@
# Changes in version 2.?.? - 2019-??-??
+ * Medium changes
+ - Make NetworkStatusEntryImpl#parseSLine thread-safe.
+
# Changes in version 2.8.0 - 2019-10-18
diff --git a/src/main/java/org/torproject/descriptor/impl/NetworkStatusEntryImpl.java b/src/main/java/org/torproject/descriptor/impl/NetworkStatusEntryImpl.java
index 2ddaaa0..e6a78f6 100644
--- a/src/main/java/org/torproject/descriptor/impl/NetworkStatusEntryImpl.java
+++ b/src/main/java/org/torproject/descriptor/impl/NetworkStatusEntryImpl.java
@@ -12,7 +12,6 @@ import org.torproject.descriptor.NetworkStatusEntry;
import java.util.ArrayList;
import java.util.BitSet;
import java.util.EnumSet;
-import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -48,13 +47,19 @@ public class NetworkStatusEntryImpl implements NetworkStatusEntry {
return lines;
}
+ private Map<String, Integer> flagIndexes;
+
+ private Map<Integer, String> flagStrings;
+
protected NetworkStatusEntryImpl(DescriptorImpl parent, int offset,
- int length, boolean microdescConsensus)
- throws DescriptorParseException {
+ int length, boolean microdescConsensus, Map<String, Integer> flagIndexes,
+ Map<Integer, String> flagStrings) throws DescriptorParseException {
this.parent = parent;
this.offset = offset;
this.length = length;
this.microdescConsensus = microdescConsensus;
+ this.flagIndexes = flagIndexes;
+ this.flagStrings = flagStrings;
this.parseStatusEntryBytes();
this.clearAtMostOnceKeys();
}
@@ -160,21 +165,17 @@ public class NetworkStatusEntryImpl implements NetworkStatusEntry {
this.orAddresses.add(parts[1]);
}
- private static Map<String, Integer> flagIndexes = new HashMap<>();
-
- private static Map<Integer, String> flagStrings = new HashMap<>();
-
private void parseSLine(String[] parts)
throws DescriptorParseException {
this.parsedAtMostOnceKey(Key.S);
- BitSet flags = new BitSet(flagIndexes.size());
+ BitSet flags = new BitSet(this.flagIndexes.size());
for (int i = 1; i < parts.length; i++) {
String flag = parts[i];
- if (!flagIndexes.containsKey(flag)) {
- flagStrings.put(flagIndexes.size(), flag);
- flagIndexes.put(flag, flagIndexes.size());
+ if (!this.flagIndexes.containsKey(flag)) {
+ this.flagStrings.put(this.flagIndexes.size(), flag);
+ this.flagIndexes.put(flag, this.flagIndexes.size());
}
- flags.set(flagIndexes.get(flag));
+ flags.set(this.flagIndexes.get(flag));
}
this.flags = flags;
}
@@ -353,7 +354,7 @@ public class NetworkStatusEntryImpl implements NetworkStatusEntry {
if (this.flags != null) {
for (int i = this.flags.nextSetBit(0); i >= 0;
i = this.flags.nextSetBit(i + 1)) {
- result.add(flagStrings.get(i));
+ result.add(this.flagStrings.get(i));
}
}
return result;
diff --git a/src/main/java/org/torproject/descriptor/impl/NetworkStatusImpl.java b/src/main/java/org/torproject/descriptor/impl/NetworkStatusImpl.java
index 0694aff..b763f30 100644
--- a/src/main/java/org/torproject/descriptor/impl/NetworkStatusImpl.java
+++ b/src/main/java/org/torproject/descriptor/impl/NetworkStatusImpl.java
@@ -10,7 +10,9 @@ import org.torproject.descriptor.NetworkStatusEntry;
import java.io.File;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import java.util.SortedMap;
import java.util.TreeMap;
@@ -19,6 +21,10 @@ import java.util.TreeMap;
* delegate the specific parts to the subclasses. */
public abstract class NetworkStatusImpl extends DescriptorImpl {
+ protected Map<String, Integer> flagIndexes = new HashMap<>();
+
+ protected Map<Integer, String> flagStrings = new HashMap<>();
+
protected NetworkStatusImpl(byte[] rawDescriptorBytes, int[] offsetAndLength,
File descriptorFile, boolean containsDirSourceEntries,
boolean blankLinesAllowed) throws DescriptorParseException {
@@ -140,7 +146,7 @@ public abstract class NetworkStatusImpl extends DescriptorImpl {
protected void parseStatusEntry(int offset, int length)
throws DescriptorParseException {
NetworkStatusEntryImpl statusEntry = new NetworkStatusEntryImpl(
- this, offset, length, false);
+ this, offset, length, false, this.flagIndexes, this.flagStrings);
this.statusEntries.put(statusEntry.getFingerprint(), statusEntry);
List<String> unrecognizedStatusEntryLines = statusEntry
.getAndClearUnrecognizedLines();
diff --git a/src/main/java/org/torproject/descriptor/impl/RelayNetworkStatusConsensusImpl.java b/src/main/java/org/torproject/descriptor/impl/RelayNetworkStatusConsensusImpl.java
index 904dc35..66b8eaa 100644
--- a/src/main/java/org/torproject/descriptor/impl/RelayNetworkStatusConsensusImpl.java
+++ b/src/main/java/org/torproject/descriptor/impl/RelayNetworkStatusConsensusImpl.java
@@ -119,7 +119,8 @@ public class RelayNetworkStatusConsensusImpl extends NetworkStatusImpl
protected void parseStatusEntry(int offset, int length)
throws DescriptorParseException {
NetworkStatusEntryImpl statusEntry = new NetworkStatusEntryImpl(this,
- offset, length, this.microdescConsensus);
+ offset, length, this.microdescConsensus, this.flagIndexes,
+ this.flagStrings);
this.statusEntries.put(statusEntry.getFingerprint(), statusEntry);
List<String> unrecognizedStatusEntryLines = statusEntry
.getAndClearUnrecognizedLines();
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits