0033409: Prevent race condition & improve initialization code in Mobile Server Key by org/client cache

Openbravo Issue Tracking System - Retail Modules

View Issue Details

Project

Category

View Status

Date Submitted

Last Update

0033409

Retail Modules

StoreServer

public

2016-07-01 12:25

2017-02-07 17:53

Reporter

mtaal

Assigned To

AugustoMauch

Priority

normal

Severity

minor

Reproducibility

have not tried

Status

closed

Resolution

duplicate

Platform

OS Version

Product Version

Target Version

Fixed in Version

Merge Request Status

Review Assigned To

OBNetwork customer

Support ticket

Regression level

Regression date

Regression introduced in release

Regression introduced by commit

Triggers an Emergency Pack

Summary

0033409: Prevent race condition & improve initialization code in Mobile Server Key by org/client cache

Description

The current code which initializes the cache of mobile server keys by org/client cache has a danger of a race condition as it sets the mobileServersByClient member in the beginning of the initialization code. This means that any next call to getNodeIdsForClient will continue and possibly access a not-yet-initialized mobileServersByClient.

https://code.openbravo.com/erp/pmods/org.openbravo.replication.symmetricds/file/5ff67cc8ae3b/src/org/openbravo/replication/symmetricds/util/MobileServerProvider.java#l102 [^]

Also it seems that the initialization code of the mobileServersByClient again calls getMobileServersByClient. This is not adviced in these type of initialization code.

Steps To Reproduce

See the code

Proposed Solution

To initialize a cache in safe way a code structure like this below can be used.

private Map<String, String> cache = null;

public Map<String, String> getCache() {
  if (cache == null) {
   initializeCache();
  }
  return cache;
}

private synchronized initializeCache() {
  if (cache != null) {
   return;
  }
  final Map<String, String> localCache = new HashMap...;
  // initialize the localCache, try not to call to many
  // other methods, makes it confusing, make sure that any other methods
  // you use here to initialize do not use/call getCache()

  // initialization done, set, this is threadsafe
  cache = localCache;
}

Additional Information