Thread problem in Java HashMap

Something happened that I'm not sure should work Obviously it's because I've seen it, but I need to find the root cause & I hope you can all help

We have a system for finding latitude and frequency Longitude is the postal code Instead of accessing it every time, we cache the results in a cheap memory hashtable cache, because lat& a long zip code often changes less than we publish

In any case, the hash is surrounded by a class with "get" and "add" methods, both of which are synchronized We visit this class as single

I'm not saying it's the best setting, but it's where we are (I plan to change to wrap the map in collections. Synchronizedmap() as soon as possible.)

We use this cache in a multithreaded environment, where we make 2 calls for 2 zippers (so we can calculate the distance between the two) These sometimes happen almost at the same time, so two calls are likely to access the map at the same time

Just recently, we encountered two events with different zip codes returning the same value Assuming that the initial value is actually different, is there any way to write the value to the map, which will result in writing the same value for two different keys? Or, 2 is there any way to "get" through a wire and accidentally return the same value?

My only other explanation is that the initial data is corrupted (wrong value), but it seems unlikely

Any ideas would be appreciated Thank you, Peter

(PS: please let me know if you need more information, codes, etc.)

public class InMemoryGeocodingCache implements GeocodingCache
{

private Map cache = new HashMap();
private static GeocodingCache instance = new InMemoryGeocodingCache();

public static GeocodingCache getInstance()
{
    return instance;
}

public synchronized LatLongPair get(String zip)
{
    return (LatLongPair) cache.get(zip);
}

public synchronized boolean has(String zip)
{
    return cache.containsKey(zip);
}

public synchronized void add(String zip,double lat,double lon)
{
    cache.put(zip,new LatLongPair(lat,lon));
}
}


public class LatLongPair {
double lat;
double lon;

LatLongPair(double lat,double lon)
{
    this.lat = lat;
    this.lon = lon;
}

public double getLatitude()
{
    return this.lat;
}

public double getLongitude()
{
    return this.lon;
}
}

Solution

The code looks correct

The only problem is that lat and lon are package visible, so the following can be used for the same package code:

LatLongPair llp = InMemoryGeocodingCache.getInstance().get(ZIP1);
llp.lat = x;
llp.lon = y;

This obviously modifies the objects in the cache

So let lat and lon play the final

Attachment: since your zip code is unique and small, you do not need to calculate the hash value on each operation It is easier to use treemap (wrapped in collections. Synchronizedmap())

P. P.S. practical method: write tests for two threads, execute put / get operations in a never-ending loop, and verify the results each time You need a multi CPU machine