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: [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]
