You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/05/03 21:40:21 UTC
[GitHub] [solr] magibney commented on a diff in pull request #834: SOLR-16154: cancel running conf listeners on close
magibney commented on code in PR #834:
URL: https://github.com/apache/solr/pull/834#discussion_r864282482
##########
solr/core/src/java/org/apache/solr/cloud/ZkController.java:
##########
@@ -2753,18 +2757,25 @@ private boolean fireEventListeners(String zkDir) {
if (listeners != null && !listeners.isEmpty()) {
final Set<Runnable> listenersCopy = new HashSet<>(listeners);
// run these in a separate thread because this can be long running
+ final AtomicReference<Future<?>> listenerFuture = new AtomicReference<>();
Runnable work =
() -> {
- log.debug("Running listeners for {}", zkDir);
- for (final Runnable listener : listenersCopy) {
- try {
- listener.run();
- } catch (RuntimeException e) {
- log.warn("listener throws error", e);
+ try {
+ log.debug("Running listeners for {}", zkDir);
+ for (final Runnable listener : listenersCopy) {
+ try {
+ listener.run();
+ } catch (RuntimeException e) {
+ log.warn("listener throws error", e);
+ }
}
+ } finally {
+ futures.remove(listenerFuture.getAndSet(null));
}
};
- cc.getCoreZkRegisterExecutorService().submit(work);
+ Future<?> future = cc.getCoreZkRegisterExecutorService().submit(work);
+ listenerFuture.set(future);
+ futures.add(future);
Review Comment:
Is there a (unlikely) race condition here where a task could execute and remove its own (as yet null) Future from `futures` before the main thread has added the Future to `futures`? Could cause stale Futures to accumulate in `futures` and never get cleared out.
Perhaps making `work` a `FutureTask<Void>` would allow to add it to `futures` and set the AtomicReference _before_ submitting it to the executor?
I like this approach though!
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org