muse-dev[bot] commented on a change in pull request #2066: URL: https://github.com/apache/lucene-solr/pull/2066#discussion_r519994863
########## File path: solr/core/src/java/org/apache/solr/core/SolrCores.java ########## @@ -62,55 +51,44 @@ // to essentially queue them up to be handled via pendingCoreOps. private static final List<SolrCore> pendingCloses = new ArrayList<>(); - private TransientSolrCoreCacheFactory transientCoreCache; + private TransientSolrCoreCacheFactory transientSolrCoreCacheFactory = TransientSolrCoreCacheFactory.NO_OP; - private TransientSolrCoreCache transientSolrCoreCache = null; - SolrCores(CoreContainer container) { this.container = container; } protected void addCoreDescriptor(CoreDescriptor p) { synchronized (modifyLock) { if (p.isTransient()) { - if (getTransientCacheHandler() != null) { - getTransientCacheHandler().addTransientDescriptor(p.getName(), p); - } else { - log.warn("We encountered a core marked as transient, but there is no transient handler defined. This core will be inaccessible"); - } + getTransientCacheHandler().addTransientDescriptor(p.getName(), p); } else { - residentDesciptors.put(p.getName(), p); + residentDescriptors.put(p.getName(), p); } } } protected void removeCoreDescriptor(CoreDescriptor p) { synchronized (modifyLock) { if (p.isTransient()) { - if (getTransientCacheHandler() != null) { - getTransientCacheHandler().removeTransientDescriptor(p.getName()); - } + getTransientCacheHandler().removeTransientDescriptor(p.getName()); } else { - residentDesciptors.remove(p.getName()); + residentDescriptors.remove(p.getName()); } } } public void load(SolrResourceLoader loader) { - transientCoreCache = TransientSolrCoreCacheFactory.newInstance(loader, container); + transientSolrCoreCacheFactory = TransientSolrCoreCacheFactory.newInstance(loader, container); } + // We are shutting down. You can't hold the lock on the various lists of cores while they shut down, so we need to // make a temporary copy of the names and shut them down outside the lock. protected void close() { waitForLoadingCoresToFinish(30*1000); Collection<SolrCore> coreList = new ArrayList<>(); - - TransientSolrCoreCache transientSolrCoreCache = getTransientCacheHandler(); - // Release observer - if (transientSolrCoreCache != null) { - transientSolrCoreCache.close(); - } + // Release transient core cache. + getTransientCacheHandler().close(); Review comment: *THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `SolrCores.close()` indirectly reads without synchronization from `this.transientSolrCoreCacheFactory`. Potentially races with write in method `SolrCores.load(...)`. Reporting because this access may occur on a background thread. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org