3

I am considering using EnumMap in a concurrent environment. However, the environment is atypical, here's why:

  • EnumMap is always full: there are no unmapped keys when the map is exposed to the concurrent environment
  • Only put() and get() operations will be used (no iterating over, no remove(), etc.)
  • It is completely acceptable if a call to get() does not reflect a call to put() immediately or orderly.

From what I could gather, including relevant method source code, this seems to be a safe scenario (unlike if iterations were allowed). Is there anything I might have overlooked?

Kirby
  • 15,127
  • 10
  • 89
  • 104
Tadas S
  • 1,955
  • 19
  • 33

3 Answers3

4

Straight from the JavaDoc:

Like most collection implementations EnumMap is not synchronized. If multiple threads access an enum map concurrently, and at least one of the threads modifies the map, it should be synchronized externally. This is typically accomplished by synchronizing on some object that naturally encapsulates the enum map. If no such object exists, the map should be "wrapped" using the Collections.synchronizedMap(java.util.Map<K, V>) method. This is best done at creation time, to prevent accidental unsynchronized access:

 Map<EnumKey, V> m = Collections.synchronizedMap(new EnumMap<EnumKey, V>(...));
Adam Siemion
  • 15,569
  • 7
  • 58
  • 92
  • There are no unwanted side effects in the scenario I described. Surely, the puts() will not be acknowledged in any logical fashion, but they do not need to be :) – Tadas S Jan 21 '14 at 14:15
4

In general, using non-thread-safe classes across threads is fraught with many problems. In your particular case, assuming safe publication after all keys have had values assigned (such that map.size() == TheEnum.values().length), the only problem I can see from a quickish glance of EnumMap's code in Java 1.6 is that a put may not ever get reflected in another thread. But that's only true because of the internals of EnumMap's implementation, which could change in the future. In other words, future changes could break the use case in more dangerous, subtle ways.

It's possible to write correct code that still contains data races -- but it's tricky. Why not just wrap the instance in a Collections.synchronizedMap?

yshavit
  • 42,327
  • 7
  • 87
  • 124
  • 1
    Indeed, basing assumptions on specific JDK's internals is not a good thing to do. – Tadas S Jan 21 '14 at 14:13
  • Make sure to read Tim B's warning, below -- I forgot to add it. Specifically, that if you put object A in one thread, and read it in another, you may read a partial view of A. In order for this to work, the A itself must be threadsafe _even if unsafely published_. – yshavit Jan 21 '14 at 19:51
0

The problem you have is that threads may not ever see the change made by another thread or they may see partially made changes. It's the same reason double-check-locking was broken before java 5 introduced volatile.

It might work if you made the EnumMap reference volatile but I'm not 100% sure even then, you might need the internal references inside the EnumMap to be volatile and obviously you can't do that without doing your own version of EnumMap.

Tim B
  • 40,716
  • 16
  • 83
  • 128
  • "That threads may not ever see the change made by another thread or they may see partially made changes." - this is acceptable; namely, that some put()s may be arbitrarily delayed (while others not) is not a problem. I am exposing a concrete EnumMap instance, so your comments about `volatile` are irrelevant. – Tadas S Jan 21 '14 at 14:16
  • 3
    They may **never** see the change. More importantly the value you `put` into the map may only be partially initialized at the point another thread starts viewing it! The compiler/JVM is allowed quite a lot of freedom about changing bits of code around. – Tim B Jan 21 '14 at 15:08