bruno-roustant commented on code in PR #3283: URL: https://github.com/apache/solr/pull/3283#discussion_r2010382686
########## solr/core/src/java/org/apache/solr/core/CoreContainer.java: ########## @@ -2323,16 +2323,14 @@ public SolrCore getCore(String name, UUID id) { // TestConfigSetsAPI and TestLazyCores if (desc == null || zkSys.getZkController() != null) return null; - // This will put an entry in pending core ops if the core isn't loaded. Here's where moving the - // waitAddPendingCoreOps to createFromDescriptor would introduce a race condition. - core = solrCores.waitAddPendingCoreOps(name); - - if (isShutDown) { - // We're quitting, so stop. This needs to be after the wait above since we may come off the - // wait as a consequence of shutting down. - return null; - } try { + // This will put an entry in pending core ops if the core isn't loaded. Here's where moving + // the waitAddPendingCoreOps to createFromDescriptor would introduce a race condition. + core = solrCores.waitAddPendingCoreOps(name); Review Comment: You're right. I wanted to have a consistent way to try-finally between all the calls to waitAddPendingCoreOps. That means that changing waitAddPendingCoreOps to throw an exception is either not appropriate, or needs more care with the other callers. And btw, the other callers may have currently an issue if an exception is thrown inside waitAddPendingCoreOps. -- 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. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org