From 187f7f6d4e628bf4ae103d0e3dcb1459e56308bd Mon Sep 17 00:00:00 2001 From: Ivan Hrasko Date: Tue, 29 Oct 2024 09:39:53 +0100 Subject: [PATCH] Remove LightyModule#startBlocking Remove practically unused LightyModule#startBlocking method which helps us to get rid of CountDownLatch in AbstractLightyModule. This method seems to be mixing concepts of using Future and CountDownLatch synchronisation tools. IMO there is no need to block start method. Additionally, there were some unusual usages in tests. They were trying to invoke a blocking call and wrap its result into Future. JIRA: LIGHTY-299 Signed-off-by: Ivan Hrasko --- .../controller/api/AbstractLightyModule.java | 41 +----------- .../core/controller/api/LightyModule.java | 7 --- .../core/controller/api/LightyModuleTest.java | 63 ------------------- 3 files changed, 2 insertions(+), 109 deletions(-) diff --git a/lighty-core/lighty-controller/src/main/java/io/lighty/core/controller/api/AbstractLightyModule.java b/lighty-core/lighty-controller/src/main/java/io/lighty/core/controller/api/AbstractLightyModule.java index ef7ea0e274..ab1cb4721a 100644 --- a/lighty-core/lighty-controller/src/main/java/io/lighty/core/controller/api/AbstractLightyModule.java +++ b/lighty-core/lighty-controller/src/main/java/io/lighty/core/controller/api/AbstractLightyModule.java @@ -7,32 +7,27 @@ */ package io.lighty.core.controller.api; -import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; -import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; -import java.util.function.Consumer; import javax.servlet.ServletException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** * This abstract class implement {@link LightyModule} interface with - * synchronization of {@link LightyModule#start()}, - * {@link LightyModule#startBlocking()} and {@link LightyModule#shutdown()} + * synchronization of {@link LightyModule#start()} and {@link LightyModule#shutdown()} * methods. Users who don't want to implement their own synchronization * can extend this class and provide just * {@link AbstractLightyModule#initProcedure()} and * {@link AbstractLightyModule#stopProcedure()} methods.These methods * will be then automatically called in - * {@link AbstractLightyModule#start()}, - * {@link AbstractLightyModule#startBlocking()} and + * {@link AbstractLightyModule#start()} and * {@link AbstractLightyModule#shutdown()} methods. * *

@@ -63,7 +58,6 @@ */ public abstract class AbstractLightyModule implements LightyModule { private static final Logger LOG = LoggerFactory.getLogger(AbstractLightyModule.class); - private final CountDownLatch shutdownLatch; private ListeningExecutorService executorService; private boolean executorIsProvided; private volatile boolean running; @@ -77,7 +71,6 @@ public AbstractLightyModule(final ExecutorService executorService) { this.executorService = MoreExecutors.listeningDecorator(executorService); this.executorIsProvided = true; } - this.shutdownLatch = new CountDownLatch(1); this.running = false; } @@ -127,35 +120,6 @@ public synchronized ListenableFuture start() { }); } - @Override - public void startBlocking() throws InterruptedException { - LOG.info("Start LightyModule {} and block until it will shutdown.", this.getClass().getSimpleName()); - start(); - LOG.debug("Waiting for LightyModule {} shutdown.", this.getClass().getSimpleName()); - this.shutdownLatch.await(); - LOG.info("LightyModule {} shutdown complete. Stop blocking.", this.getClass().getSimpleName()); - } - - /** - * Start and block until shutdown is requested. - * @param initFinishCallback callback that will be called after start is completed. - * @throws InterruptedException thrown in case module initialization fails. - */ - public void startBlocking(final Consumer initFinishCallback) throws InterruptedException { - Futures.addCallback(start(), new FutureCallback() { - @Override - public void onSuccess(final Boolean result) { - initFinishCallback.accept(true); - } - - @Override - public void onFailure(final Throwable cause) { - initFinishCallback.accept(false); - } - }, MoreExecutors.directExecutor()); - this.shutdownLatch.await(); - } - @Override public synchronized ListenableFuture shutdown() { if (!this.running) { @@ -191,7 +155,6 @@ public synchronized ListenableFuture shutdown() { @Override public final boolean shutdown(final long duration, final TimeUnit unit) { try { - shutdownLatch.countDown(); final var stopSuccess = shutdown().get(duration, unit); if (stopSuccess) { LOG.info("LightyModule {} stopped successfully!", getClass().getSimpleName()); diff --git a/lighty-core/lighty-controller/src/main/java/io/lighty/core/controller/api/LightyModule.java b/lighty-core/lighty-controller/src/main/java/io/lighty/core/controller/api/LightyModule.java index c88b5458e8..494286c7db 100644 --- a/lighty-core/lighty-controller/src/main/java/io/lighty/core/controller/api/LightyModule.java +++ b/lighty-core/lighty-controller/src/main/java/io/lighty/core/controller/api/LightyModule.java @@ -26,13 +26,6 @@ public interface LightyModule { */ ListenableFuture start(); - /** - * Start and block until shutdown is requested. - * - * @throws InterruptedException thrown in case module initialization fails. - */ - void startBlocking() throws InterruptedException; - /** * Shutdown module. * diff --git a/lighty-core/lighty-controller/src/test/java/io/lighty/core/controller/api/LightyModuleTest.java b/lighty-core/lighty-controller/src/test/java/io/lighty/core/controller/api/LightyModuleTest.java index a57c9997fd..c457489b03 100644 --- a/lighty-core/lighty-controller/src/test/java/io/lighty/core/controller/api/LightyModuleTest.java +++ b/lighty-core/lighty-controller/src/test/java/io/lighty/core/controller/api/LightyModuleTest.java @@ -7,13 +7,9 @@ */ package io.lighty.core.controller.api; -import com.google.common.util.concurrent.SettableFuture; import io.lighty.core.controller.impl.LightyControllerBuilder; import io.lighty.core.controller.impl.util.ControllerConfigUtils; -import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -26,7 +22,6 @@ public class LightyModuleTest { private static final long MAX_INIT_TIMEOUT = 15000L; private static final long MAX_SHUTDOWN_TIMEOUT = 15000L; - private static final long SLEEP_AFTER_SHUTDOWN_TIMEOUT = 800L; private ExecutorService executorService; private LightyModule moduleUnderTest; @@ -84,62 +79,4 @@ public void testShutdown_before_start() throws Exception { this.moduleUnderTest.shutdown(MAX_SHUTDOWN_TIMEOUT, TimeUnit.MILLISECONDS); Mockito.verify(executorService, Mockito.times(0)).execute(Mockito.any()); } - - @Test - public void testStartBlocking_and_shutdown() throws Exception { - this.moduleUnderTest = getModuleUnderTest(getExecutorService()); - startStopBlocking(this.moduleUnderTest instanceof AbstractLightyModule); - } - - @Test - public void testStartStopBlocking() throws Exception { - this.moduleUnderTest = getModuleUnderTest(getExecutorService()); - startStopBlocking(false); - } - - private void startStopBlocking(final boolean isAbstract) throws Exception { - Future startBlockingFuture; - if (isAbstract) { - startBlockingFuture = startBlockingOnLightyModuleAbstractClass(); - } else { - startBlockingFuture = startBlockingOnLightyModuleInterface(); - } - //test if thread which invokes startBlocking method is still running (it should be) - Assert.assertFalse(startBlockingFuture.isDone()); - - this.moduleUnderTest.shutdown(MAX_SHUTDOWN_TIMEOUT, TimeUnit.MILLISECONDS); - try { - //test if thread which invokes startBlocking method is done after shutdown was called - //(after small timeout due to synchronization); - startBlockingFuture.get(SLEEP_AFTER_SHUTDOWN_TIMEOUT, TimeUnit.MILLISECONDS); - } catch (TimeoutException e) { - Assert.fail("Waiting for finish of startBlocking method thread timed out. you may consider to adjust" - + "timeout by overriding SLEEP_AFTER_SHUTDOWN_TIMEOUT", e); - } - - Mockito.verify(executorService, Mockito.times(2)).execute(Mockito.any()); - } - - private Future startBlockingOnLightyModuleAbstractClass() throws ExecutionException, InterruptedException { - SettableFuture initDoneFuture = SettableFuture.create(); - Future startFuture = Executors.newSingleThreadExecutor().submit(() -> { - ((AbstractLightyModule)this.moduleUnderTest).startBlocking(initDoneFuture::set); - return true; - }); - try { - initDoneFuture.get(MAX_INIT_TIMEOUT, TimeUnit.MILLISECONDS); - } catch (TimeoutException e) { - Assert.fail("Init timed out.", e); - } - return startFuture; - } - - private Future startBlockingOnLightyModuleInterface() throws InterruptedException { - Future startFuture = Executors.newSingleThreadExecutor().submit(() -> { - this.moduleUnderTest.startBlocking(); - return true; - }); - Thread.sleep(MAX_INIT_TIMEOUT); - return startFuture; - } }