Synchronization for inverse view of synchronized BiMap

Question

The Maps.synchronizedBiMap() method states that

it is imperative that the user manually synchronize on the returned map when accessing any of its collection views.

Does this include the inverse() view of the BiMap? For example, if the variables are initialized as in the following example, can invoking inverse.put() from other threads be problematic (e.g. the change is not visible in a get() call on either map or inverse, even if put happened-before get)?

BiMap<Object, Object> map = Maps.synchronizedBiMap(HashBiMap.create());
BiMap<Object, Object> inverse = map.inverse();

If this is in fact a problem, is there a standard/recommended way of solving this?

// EDIT

Looking at the implementation, it seems like the inverse() of a SynchronizedBiMap is also a SynchronizedBiMap, sharing the same mutex. Does this mean the described problem is non-existent? Confirmation from a Guava Collections expert would be much appreciated ;)

The described problem is apparent from the example that follows the sentence you quoted: when iterating (or doing any sequence of operations) on a view, you need to synchronize, on the synchronized BiMap, the whole iteration (or sequence) in order to have deterministic behavior. — JB Nizet, May 18 '17 at 15:31
As with all the views, manual synchronization is only required when doing iteration, and it's required when iterating over the inverse as well. You don't have to synchronize e.g. keySet.contains either. — Louis Wasserman, May 19 '17 at 04:07
@LouisWasserman If you want to iterate over the inverse, should you synchronize on the inverse map or the original map? — Tavian Barnes, May 19 '17 at 18:39
Synchronize on the original map. The original map is the mutex for itself and all its views, including the inverse. — Louis Wasserman, May 19 '17 at 18:39

Grzegorz Rożniecki · Answer 1 · 2017-05-21T07:36:32.703

No, in this case you don't have to synchronize on inversed map. You cited only a fragment of the documentation, I'll also switch original keySet() with inverse() in the example code:

Returns a synchronized (thread-safe) bimap backed by the specified bimap. In order to guarantee serial access, it is critical that all access to the backing bimap is accomplished through the returned bimap.

It is imperative that the user manually synchronize on the returned map when accessing any of its collection views:
BiMap<Long, String> map = Maps.synchronizedBiMap(
    HashBiMap.<Long, String>create());
//...
BiMap<String, Long> inverse = map.inverse();  // Needn't be in synchronized block
Set<String> set = inverse.keySet();  // Needn't be in synchronized block
//...
synchronized (map) {  // Synchronizing on map, not set!
  Iterator<String> it = set.iterator(); // Must be in synchronized block
  while (it.hasNext()) {
    foo(it.next());
  }
}
Failure to follow this advice may result in non-deterministic behavior.

So when you want a deterministic behavior during iteration over its views (which includes in iterating over inverse view), you have to synchronize on your instance.

In case of .inverse(), as you mentioned, it creates new synchronized bimap using same mutex object, so it synchronizes properly on methods like get or contains.

You claim that synchronization gets you deterministic order, and if you don't need deterministic order you don't need synchronization. But that's not what the documentation says. It says non-deterministic _behavior,_ which can include [throwing random exceptions](http://www.catb.org/jargon/html/N/nasal-demons.html). You absolutely must synchronize if you want to iterate at all. — Louis Wasserman, May 19 '17 at 18:41
@LouisWasserman Fair enough, thanks for correction. I've edited my answer. — Grzegorz Rożniecki, May 21 '17 at 07:38

Synchronization for inverse view of synchronized BiMap

1 Answers1