Menu
Will Guava's Tables.newCustomTable(Map, Supplier) method return thread safe tables when supplied with thread safe maps? For example:
public static <R, C, V> Table<R, C, V> newConcurrentTable() {
return Tables.newCustomTable(
new ConcurrentHashMap<R, Map<C, V>>(),
new Supplier<Map<C, V>>() {
public Map<C, V> get() {
return new ConcurrentHashMap<C, V>();
}
});
}
Does that code actually return concurrent tables?
389
The returned table is not thread-safe or serializable, even if the underlying table is.
TreeMap and TreeSet are not thread-safe collections, so care must be taken to ensure when used in multi-threaded programs. Both TreeMap and TreeSet are safe when read, even concurrently, by multiple threads.
Immutable map is born to thread-safe. You could use ImmutableMap of Guava. Show activity on this post. In short, no you don't need the map to be thread-safe if the reads are non-destructive and the map reference is safely published to the client.
Maps are naturally one of the most widely style of Java collection. And, importantly, HashMap is not a thread-safe implementation, while Hashtable does provide thread-safety by synchronizing operations. Even though Hashtable is thread safe, it is not very efficient. Another fully synchronized Map, Collections.
From the doc: "If multiple threads access this table concurrently and one of the threads modifies the table, it must be synchronized externally."
Concurrent backing collections aren't enough.
127
Kevin Bourrillion is right. The technical reason for the map you've constructed not to be thread safe is that even if the maps you are using are thread safe, the table operations may not be. Let me give an example of put, as implemented in the StandardTable, which is used by Tables.newCustomTable:
public V put(R rowKey, C columnKey, V value) {
Map<C, V> map = backingMap.get(rowKey);
if (map == null) {
map = factory.get();
backingMap.put(rowKey, map);
}
return map.put(columnKey, value);
}
Thread safety is compromised in the handling of the map == null case. Namely, two or more threads could enter that block and create a new entry for the columnKey and the last one to perform a backingMap.put(rowKey, map) would ultimately override the entry for the columnKey in the backingMap, which would lead to the loss of put operations performed by other threads. In particular the result of this operation in a multithreaded environment is non-deterministic, which is equivalent to saying that this operation is not thread safe.
The correct implementation of this method would be:
public V put(R rowKey, C columnKey, V value) {
ConcurrentMap<C, V> map = table.get(rowKey);
if (map == null) {
backingMap.putIfAbsent(rowKey, factory.get());
}
map = backingMap.get(rowKey);
return map.put(columnKey, value);
}
I'm currently investigating if it is possible to use the ForwardingTable implementation together with what you've wanted to do, to get a properly thread safe ConcurrentTable.
But to be honest, I think the reason there is no thread-safe implementation of the Table is that the interface itself doesn't provide any concurrency constructs, such as putIfAbsent or replace.
22
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With
