gus-asf commented on code in PR #4065:
URL: https://github.com/apache/solr/pull/4065#discussion_r2709789838
##########
solr/core/src/java/org/apache/solr/servlet/CoreContainerAwareHttpFilter.java:
##########
@@ -29,11 +29,10 @@
public abstract class CoreContainerAwareHttpFilter extends HttpFilter {
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
- private CoreContainerProvider containerProvider;
Review Comment:
- I'm not at all worried about init() in a full running jetty webapp.
- I'm also not worried about order of operations in the tests.
- I am worried that our test infrastructure may not have perfectly ensured
"happens before" and making this volatile is a simple way of ensuring that
guarantee.
- I strongly suspect the folks who wrote jetty have thought carefully about
"happens before" with respect to various phases of the servlet lifecycle, so I
view this as a test-only accommodation.
I believe I did this after seeing null in a test run. The explanation seemed
to be that a thread in the test wasn't seeing the change to this field
(presumably because it hadn't been forced to sync by a coordinated lock on a
monitor). I wrote all this code just before we stopped for a community discuss
thread, so the exact detail is now a bit foggy to me. I will remove and run the
tests a few of times... maybe I'll catch it in the act again.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
