psalagnac commented on code in PR #3283:
URL: https://github.com/apache/solr/pull/3283#discussion_r2010360147
##########
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:
This is not safe to move this call inside the `try` block.
If an exception is raised for whatever reason, it means we did not have the
lock. In this case, we should **not** execute the finally block that releases
the lock.
--
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
