Skip to content

Commit

Permalink
Remove LightyModule#startBlocking
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@ihrasko
ihrasko committed Oct 30, 2024
1 parent 02c9bff commit af62885
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 109 deletions.
41 changes: 2 additions & 39 deletions ...e/lighty-controller/src/main/java/io/lighty/core/controller/api/AbstractLightyModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
* <p>
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down Expand Up @@ -127,35 +120,6 @@ public synchronized ListenableFuture<Boolean> 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<Boolean> initFinishCallback) throws InterruptedException {
Futures.addCallback(start(), new FutureCallback<Boolean>() {
@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<Boolean> shutdown() {
if (!this.running) {
Expand Down Expand Up @@ -191,7 +155,6 @@ public synchronized ListenableFuture<Boolean> 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());
Expand Down
7 changes: 0 additions & 7 deletions lighty-core/lighty-controller/src/main/java/io/lighty/core/controller/api/LightyModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,6 @@ public interface LightyModule {
*/
ListenableFuture<Boolean> start();

/**
* Start and block until shutdown is requested.
*
* @throws InterruptedException thrown in case module initialization fails.
*/
void startBlocking() throws InterruptedException;

/**
* Shutdown module.
*
Expand Down
63 changes: 0 additions & 63 deletions ...-core/lighty-controller/src/test/java/io/lighty/core/controller/api/LightyModuleTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<Boolean> 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<Boolean> startBlockingOnLightyModuleAbstractClass() throws ExecutionException, InterruptedException {
SettableFuture<Boolean> initDoneFuture = SettableFuture.create();
Future<Boolean> 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<Boolean> startBlockingOnLightyModuleInterface() throws InterruptedException {
Future<Boolean> startFuture = Executors.newSingleThreadExecutor().submit(() -> {
this.moduleUnderTest.startBlocking();
return true;
});
Thread.sleep(MAX_INIT_TIMEOUT);
return startFuture;
}
}

0 comments on commit af62885

Please sign in to comment.