Synchronized LinkedHashed map I've written the following code ages ago (10+ years) which is part of a simple chat server. I'm going to refactor it a bit in Java and then for fun I'm going to convert it to Scala and use Akka actors instead of threading.

Do you see any thread issues that can arise from the method of synchronization?

If a thread appends a record and another thread performs a read on the thread that reads, is it not going to have the current data?

Upon reading this article, I assume that is not the case?

I could use a ConcurrentHashMap instead but it doesn't preserve the order of insertion.

final class UserRecord
{
    //unsynchronized version of hashtable which maintains the order
    //which the objects where inserted.
    private LinkedHashMap container;

    public UserRecord()
    {
        container = new LinkedHashMap();
    }

    //return true if the user name is not taken
    synchronized boolean add(ClientHandler handler)
    {
        if (container.containsKey(handler.getUserName()) == true)
            return false;

        container.put(handler.getUserName(), handler);
        return true;
    }

    synchronized void remove(ClientHandler handler)
    {
        container.remove(handler.getUserName());
    }

    synchronized int size()
    {
        return container.values().size();
    }

    synchronized ClientHandler getHandler(String userName)
    {
        return (ClientHandler) container.get(userName);
    }

    synchronized Iterator getIterator()
    {
        return container.values().iterator();
    }

    synchronized void clear()
    {
        this.container.clear();
    }
}
Yes there is a thread safety issue. While the getIterator() is synchronized, the Iterator it returns is not. LinkedHashMap supports removal through the values() view, and hence the Iterator can be used to modify the Map without proper synchronization.

  UserRecord userRecord = new UserRecord();
  populate(userRecord);

  Iterator iterator = userRecord.getIterator();

  iterator.next();
  iterator.remove(); // unsynchronized modification of the Map


The easiest solution is to use Collections.synchronizedMap() which basically does what you attempted. 

private Map&lt;String, ClientHandler&gt; container;

public UserRecord()
{
    container = Collections.synchronizedMap(new LinkedHashMap&lt;&gt;());
}


If you do this, the methods on UserRecord no longer need to be synchronized.

As an aside, you will want to properly use generics.