javadesign-patternsmultiton

Thread-safe multiton pattern


Inspired by a comment to an given answer I tried to create a thread-safe implementation of the multiton pattern, which relies on unique keys and performs locks on them (I have the idea from JB Nizet's answer on this question).

Question

Is the implementation I provided viable?

I'm not interested in whether Multiton (or Singleton) are in general good patterns, it would result in a discussion. I just want a clean and working implementation.

Contras:

Pros


public class Multiton
{
  private static final Map<Enum<?>, Multiton> instances = new HashMap<Enum<?>, Multiton>();

  private Multiton() {System.out.println("Created instance."); }

  /* Can be called concurrently, since it only synchronizes on id */
  public static <KEY extends Enum<?> & MultitionKey> Multiton getInstance(KEY id)
  {
    synchronized (id)
    {
      if (instances.get(id) == null)
        instances.put(id, new Multiton());
    }
    System.out.println("Retrieved instance.");
    return instances.get(id);
  }

  public interface MultitionKey { /* */ }

  public static void main(String[] args) throws InterruptedException
  {
    //getInstance(Keys.KEY_1);
    getInstance(OtherKeys.KEY_A);

    Runnable r = new Runnable() {
      @Override
      public void run() { getInstance(Keys.KEY_1); }
    };

    int size = 100;
    List<Thread> threads = new ArrayList<Thread>();
    for (int i = 0; i < size; i++)
      threads.add(new Thread(r));

    for (Thread t : threads)
      t.start();

    for (Thread t : threads)
      t.join();
  }

  enum Keys implements MultitionKey
  {
    KEY_1;

    /* define more keys */
  }

  enum OtherKeys implements MultitionKey
  {
    KEY_A;

    /* define more keys */
  }
}

I tried to prevent the resizing of the map and the misuse of the enums I sychronize on. It's more of a proof of concept, before I can get it over with! :)

public class Multiton
{
  private static final Map<MultitionKey, Multiton> instances = new HashMap<MultitionKey, Multiton>((int) (Key.values().length/0.75f) + 1);
  private static final Map<Key, MultitionKey> keyMap;

  static
  {
    Map<Key, MultitionKey> map = new HashMap<Key, MultitionKey>();
    map.put(Key.KEY_1, Keys.KEY_1);
    map.put(Key.KEY_2, OtherKeys.KEY_A);
    keyMap = Collections.unmodifiableMap(map);
  }

  public enum Key {
    KEY_1, KEY_2;
  }

  private Multiton() {System.out.println("Created instance."); }

  /* Can be called concurrently, since it only synchronizes on KEY */
  public static <KEY extends Enum<?> & MultitionKey> Multiton getInstance(Key id)
  {
    @SuppressWarnings ("unchecked")
    KEY key = (KEY) keyMap.get(id);
    synchronized (keyMap.get(id))
    {
      if (instances.get(key) == null)
        instances.put(key, new Multiton());
    }
    System.out.println("Retrieved instance.");
    return instances.get(key);
  }

  private interface MultitionKey { /* */ }

  private enum Keys implements MultitionKey
  {
    KEY_1;

    /* define more keys */
  }

  private enum OtherKeys implements MultitionKey
  {
    KEY_A;

    /* define more keys */
  }
}

Solution

  • It is absolutely not thread-safe. Here is a simple example of the many, many things that could go wrong.

    Thread A is trying to put at key id1. Thread B is resizing the buckets table due to a put at id2. Because these have different synchronization monitors, they're off to the races in parallel.

    Thread A                      Thread B
    --------                      --------
    b = key.hash % map.buckets.size   
    
                                 copy map.buckets reference to local var
                                 set map.buckets = new Bucket[newSize]
                                 insert keys from old buckets into new buckets
    
    insert into map.buckets[b]
    

    In this example, let's say Thread A saw the map.buckets = new Bucket[newSize] modification. It's not guaranteed to (since there's no happens-before edge), but it may. In that case, it'll be inserting the (key, value) pair into the wrong bucket. Nobody will ever find it.

    As a slight variant, if Thread A copied the map.buckets reference to a local var and did all its work on that, then it'd be inserting into the right bucket, but the wrong buckets table; it wouldn't be inserting into the new one that Thread B is about to install as the table for everyone to see. If the next operation on key 1 happens to see the new table (again, not guaranteed to but it may), then it won't see Thread A's actions because they were done on a long-forgotten buckets array.