dsmiley commented on code in PR #2474: URL: https://github.com/apache/solr/pull/2474#discussion_r1610765299
########## solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java: ########## @@ -93,69 +87,69 @@ public class CoreContainerProvider implements ServletContextListener { private HttpClient httpClient; private SolrMetricManager metricManager; private RateLimitManager rateLimitManager; - private final CountDownLatch init = new CountDownLatch(1); private String registryName; - // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might - // have multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real - // server. - private static final Map<ContextInitializationKey, ServiceHolder> services = - Collections.synchronizedMap(new WeakHashMap<>()); - - // todo: dependency injection instead, but for now this method and the associated map will have - // to suffice. - // Note that this relies on ServletContext.equals() not implementing anything significantly - // different than Object.equals for its .equals method (I've found no implementation that even - // implements it). - public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException { - ContextInitializationKey key = new ContextInitializationKey(ctx); - return services.computeIfAbsent(key, ServiceHolder::new); + + /** Acquires an instance from the context, waiting if necessary. */ + public static CoreContainerProvider serviceForContext(ServletContext ctx) + throws InterruptedException { + long startWait = System.nanoTime(); + synchronized (ctx) { Review Comment: > Never know when they might also (try to) lock it during server initialization. I'm not concerned; it's only Jetty and us to worry about. The code here is resilient to `wait` being notified inexplicably. There's a loop and it keeps checking for the CoreContainer to show up. Perhaps it'll log a bit more that it's waiting; it's okay. In practice I don't think it'll wait at all since you had pointed out in comments that the ServletContextListener is initialized first. -- 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