EHC ❯ CacheConfiguration#clone method copies collection fields references instead of cloning them
-
Bug
-
Status: Resolved
-
2 Major
-
Resolution: Fixed
-
ehcache-core
-
-
ljacomet
-
Reporter: kmacleod
-
September 25, 2014
-
0
-
Watchers: 4
-
October 17, 2014
-
October 17, 2014
Description
The method {{CacheConfiguration#clone}} has a critical flaw whereby any of the list fields of {{CacheConfiguration}} which are empty are copied to the clone by reference, instead of being initialised with new empty lists. The list fields are only properly cloned if their are non-empty.
The problem is that {{CacheConfiguration#clone}} starts by using {{Object#clone}}, which does a shallow copy of each field (i.e. the clone has references to the same collections as the original). It then goes on to initialize each collection field individually, but it only does this if the collection is not empty. If the collection *is* empty, then the clone retains references to the original’s collection.
As a result, if client code mutates any of those collections on the clone’s {{CacheConfiguration}}, then it was also mutate the original. This is very bad, especially if the {{CacheConfiguration}} being cloned is the cache manager’s default configuration.
The following unit test demonstrates the bug:
@Test
public void test() throws Exception {
CacheManager cacheManager = CacheManager.create();
CacheConfiguration defaultConfig = cacheManager.getConfiguration().getDefaultCacheConfiguration();
CacheConfiguration clonedConfig = defaultConfig.clone();
clonedConfig.addCacheEventListenerFactory(new CacheConfiguration.CacheEventListenerFactoryConfiguration());
assertThat(defaultConfig.getCacheEventListenerConfigurations(), hasSize(0));
}
Comments
Kenny MacLeod 2014-09-26
Louis Jacomet Jacomet 2014-10-03
That’s indeed not normal.
Will do the necessary changes and verify they do not have unexpected side effects.
Louis Jacomet Jacomet 2014-10-17
Fixed and released in 2.8.5 which is out.
Will be in upcoming 2.7.7 and 2.9.1 - no ETAs
The solution would appear to be a simple one. In the methods that initialize the collection fields (e.g. in {{#cloneCacheEventListenerConfigurations}}), then remove the initial {{if list.size > 0}} check. This will ensure that the clone is initialized with a new collection regardless of whether the original collection is empty or not. The {{if}} check in this case appears to be a rather pointless micro-optimization.