Do I have reordering issue and is it due to reference escape?

If you want to be spec-compliant, you cannot apply this constructor:

private DataWrapper(String name) {  this.name = name;  this.data = loadData(name);  map.put(name, this);}

As you point out, the JVM is allowed to reorder this to something like:

private DataWrapper(String name) {  map.put(name, this);  this.name = name;  this.data = loadData(name);}

When assigning a value to final field, this implies a so-called freeze action at the end of the constructor. The memory model guarantees a happens before relationship between this freeze action and any dereferenzation of the instance on which this freeze action was applied upon. This relationship does however only exist at the end of the constructor, therefore, you break this relationship. By dragging the publication out of your constructor, you can fix this realtionship.

If you want a more formal overview of this relationship, I recommend looking through this slide set. I also explained the relationship in this presentation starting at about minute 34.

The implementation has some very subtle caveats.

It seems you are aware, but just to be clear,in this piece of code multiple threads may get a null instance and enter the if block,unnecessarily creating new DataWrapper instances:

public static DataWrapper getInstance(String name) {    DataWrapper instance = map.get(name);    if (instance == null) {        instance = new DataWrapper(name);    }    return instance.cloneInstance();}

It seems you are ok with that,but that requires the assumption that loadData(name) (used by DataWrapper(String)) will always return the same value.If it may return different values depending on the timing,there's no guarantee that the last thread to load the data will store it in the map, so the value may be outdated.If you say this won't happen or it's not important,that's fine, but this assumption should be at least documented.

To demonstrate another subtle issue, let me inline the instance.cloneInstance() method:

public static DataWrapper getInstance(String name) {    DataWrapper instance = map.get(name);    if (instance == null) {        instance = new DataWrapper(name);    }    return new DataWrapper(instance);}

The subtle issue here is that this return statement is not safe publication.The new DataWrapper instance may be partially constructed,and a thread may observe it in an inconsistent state,for example the fields of the object may not be set yet.

There is a simple fix for this:if you make the name and data fields final,the class becomes immutable.Immutable classes enjoy special initialization guarantees,and the return new DataWrapper(this); becomes safe publication.

With this simple change, and assuming that you're ok with the first point (loadData is not time-sensitive), I think the implementation should work correctly.

I would recommend an additional improvement that is not related to correctness, but other good practices.The current implementation has too many responsibilities:it's a wrapper around Data, and at the same time a cache.The added responsibility makes it a bit confusing to read.And as an aside, the concurrent hash map is not really used to its potential.

If you separate the responsibilities, the result can be simpler, better, and easier to read:

class DataWrapperCache {  private static final ConcurrentMap<String, DataWrapper> map = new ConcurrentHashMap<>();  public static DataWrapper get(String name) {    return map.computeIfAbsent(name, DataWrapper::new).defensiveCopy();  }}class DataWrapper {  private final String name;  private final Data data;  DataWrapper(String name) {    this.name = name;    this.data = loadData(name);  // A heavy method  }  private DataWrapper(DataWrapper that) {    this.name = that.name;    this.data = that.data.cloneInstance();  }  public DataWrapper defensiveCopy() {    return new DataWrapper(this);  }}