> On Aug. 24, 2016, 8:27 p.m., Bruce Schuchardt wrote:
> > While it's okay to synchronize like that in checkExistingCache() I think it
> > might be more appropriate to synchronize on CacheFactory.class in
> > basicCreate() and fold the checkExistingClass() method into that method
> > since that's the only place it's used. I'm concerned that the static
> > "instance" variable is being set in GemFireCacheImpl.initialize() outside
> > of synchronization. It used to be that there was only one path to
> > basicCreate() and that method synchronized on CacheFactory.class, but now
> > there appear to be other paths to it, like client-cache creation, that
> > don't synchronize.
>
> Hitesh Khamesra wrote:
> >>I'm concerned that the static "instance" variable is being set in
> GemFireCacheImpl.initialize() outside of synchronization.
> This is now part of synchronization
>
> synchronized (GemFireCacheImpl.class) {
> GemFireCacheImpl instance = checkExistingCache(existingOk,
> cacheConfig);
> if (instance == null) {
> instance = new GemFireCacheImpl(isClient, pf, system,
> cacheConfig, asyncEventListeners, typeRegistry);
> instance.initialize();
> }
> return instance;
> }
Yes, like that
- Bruce
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51385/#review146705
-----------------------------------------------------------
On Aug. 24, 2016, 6:30 p.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51385/
> -----------------------------------------------------------
>
> (Updated Aug. 24, 2016, 6:30 p.m.)
>
>
> Review request for geode, Bruce Schuchardt and Darrel Schneider.
>
>
> Repository: geode
>
>
> Description
> -------
>
> 1. When cache is closed need to return null from "checkExistingCache"
> 2. Need to synchronize cache creation
>
>
> Diffs
> -----
>
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java
> 76a7bad
>
> Diff: https://reviews.apache.org/r/51385/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
