Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Container startup issue fixed. #1912

Merged
merged 3 commits into from
Jun 2, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ static synchronized void reset() {
for (Runnable resetListener : RESET_LISTENERS) {
resetListener.run();
}
HelidonCdiProvider.unset();
RESET_LISTENERS.clear();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,18 @@ class HelidonCdiProvider implements CDIProvider {

@Override
public CDI<Object> getCDI() {
return CURRENT_CDI.get();
CDI<Object> cdi = CURRENT_CDI.get();
if (null == cdi) {
throw new IllegalStateException("There is no CDI instance available");
}
return cdi;
}

static void setCdi(CDI<Object> cdi) {
CURRENT_CDI.set(cdi);
}

static void unset() {
CURRENT_CDI.set(null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,18 @@ public SeContainer start() {
return cdi;
}
LogConfig.configureRuntime();
Contexts.runInContext(ROOT_CONTEXT, this::doStart);
try {
Contexts.runInContext(ROOT_CONTEXT, this::doStart);
} catch (Exception e) {
try {
// we must clean up
shutdown();
} catch (Exception exception) {
e.addSuppressed(exception);
}
throw e;
}

if (EXIT_ON_STARTED) {
exitOnStarted();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
* Copyright (c) 2020 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.helidon.microprofile.cdi;

import java.util.Map;

import javax.annotation.Priority;
import javax.enterprise.context.ApplicationScoped;
import javax.enterprise.context.Initialized;
import javax.enterprise.event.Observes;
import javax.enterprise.inject.se.SeContainerInitializer;
import javax.enterprise.inject.spi.CDI;
import javax.enterprise.inject.spi.Extension;

import io.helidon.config.mp.MpConfigSources;

import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.ConfigProvider;
import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import static javax.interceptor.Interceptor.Priority.PLATFORM_AFTER;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;

/**
* Unit test for problem reported in Github issue #1446.
* <p>
* Helidon container starts even if exception is thrown during initialization(after deployment)
*/
public class Gh1446Test {
private static Config originalConfig;
private static ConfigProviderResolver configResolver;
private static ClassLoader cl;

@BeforeAll
static void initClass() {
originalConfig = ConfigProvider.getConfig();
configResolver = ConfigProviderResolver.instance();
cl = Thread.currentThread().getContextClassLoader();

Config config = configResolver.getBuilder()
.withSources(MpConfigSources.create(Map.of("mp.initializer.allow", "true",
"mp.initializer.no-warn", "true")))
.build();

configResolver.registerConfig(config, cl);
}

@AfterAll
static void destroyClass() {
configResolver.registerConfig(originalConfig, cl);
}
@Test
void testStartupFails() {

RuntimeException thrown = assertThrows(RuntimeException.class, () -> SeContainerInitializer.newInstance()
.addExtensions(new FailingExtension())
.disableDiscovery()
.initialize());

thrown.printStackTrace();
assertThat(thrown.getMessage(), is("Survive this!"));

assertThrows(IllegalStateException.class, CDI::current, "There should be no CDI instance available");

}

private static class FailingExtension implements Extension {
void init(@Observes @Priority(PLATFORM_AFTER + 101) @Initialized(ApplicationScoped.class) Object e) {
throw new RuntimeException("Survive this!");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.hamcrest.CoreMatchers.instanceOf;
Expand All @@ -51,8 +52,9 @@ static void initClass() {
}

@AfterEach
@BeforeEach
void resetConfig() {
// restore the config to default
// restore the config to default (also before first test, as other tests may have broken it)
configResolver.registerConfig(defaultConfig, cl);
}

Expand Down Expand Up @@ -88,7 +90,7 @@ void testRestart2() {
.withSources(MpConfigSources.create(Map.of(HelidonContainerInitializer.CONFIG_ALLOW_INITIALIZER, "true")))
.build();

configResolver.registerConfig((org.eclipse.microprofile.config.Config) config, cl);
configResolver.registerConfig(config, cl);
// now we can start using SeContainerInitializer
SeContainerInitializer seContainerInitializer = SeContainerInitializer.newInstance();
assertThat(seContainerInitializer, instanceOf(HelidonContainerInitializer.class));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.util.LinkedList;
import java.util.List;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Supplier;
import java.util.logging.Logger;

Expand Down Expand Up @@ -133,8 +132,6 @@ final class Builder {

// this constant is to ensure we initialize Helidon CDI at build time
private static final HelidonContainer CONTAINER = HelidonContainer.instance();

static final AtomicBoolean IN_PROGRESS_OR_RUNNING = new AtomicBoolean();
private static final Logger STARTUP_LOGGER = Logger.getLogger("io.helidon.microprofile.startup.builder");

private final List<Class<?>> resourceClasses = new LinkedList<>();
Expand All @@ -148,11 +145,29 @@ final class Builder {
private boolean retainDiscovered = false;

private Builder() {
if (!IN_PROGRESS_OR_RUNNING.compareAndSet(false, true)) {
throw new IllegalStateException("There is another builder in progress, or another Server running. "
+ "You cannot run more than one in parallel");
}
LOGGER.finest(() -> "Container context id: " + CONTAINER.context().id());

ServerCdiExtension server = null;
try {
server = CDI.current()
.getBeanManager()
.getExtension(ServerCdiExtension.class);
} catch (IllegalStateException ignored) {
// CDI is not started, so we should be fine
}

if (null != server) {
if (server.started()) {
throw new IllegalStateException("Server is already started. Maybe you have initialized CDI yourself? "
+ "If you do so, then you cannot use Server.builder() to set up "
+ "your server. "
+ "Config is initialized with defaults or using "
+ "meta-config.yaml; applications are discovered using CDI. "
+ "To use custom configuration, you can use "
+ "ConfigProviderResolver.instance().registerConfig(config, "
+ "classLoader);");
}
}
}

/**
Expand All @@ -162,32 +177,21 @@ private Builder() {
* @throws MpException in case the server fails to be created
*/
public Server build() {
// make sure server is not already running
try {
ServerCdiExtension server = CDI.current()
.getBeanManager()
.getExtension(ServerCdiExtension.class);
// we may have shutdown the original instance, this is to ensure we use the current CDI.
HelidonContainer instance = HelidonContainer.instance();

if (server.started()) {
SeContainer container = (SeContainer) CDI.current();
container.close();
throw new MpException("Server is already started. Maybe you have initialized CDI yourself? "
+ "If you do so, then you cannot use Server.builder() to set up your server. "
+ "Config is initialized with defaults or using "
+ "meta-config.yaml; applications are discovered using CDI. "
+ "To use custom configuration, you can use "
+ "ConfigProviderResolver.instance().registerConfig(config, "
+ "classLoader);");
try {
// now run the build within context already
return Contexts.runInContext(instance.context(), this::doBuild);
} catch (Exception e) {
try {
((SeContainer) CDI.current()).close();
} catch (IllegalStateException ignored) {
// no cdi registered
}
} catch (IllegalStateException ignored) {
// container is not running - server cannot be started in such a case
// ignore this
}

// we may have shutdown the original instance, this is to ensure we use the current CDI.
HelidonContainer instance = HelidonContainer.instance();
// now run the build within context already
return Contexts.runInContext(instance.context(), this::doBuild);
throw e;
}
}

private Server doBuild() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Supplier;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand All @@ -49,6 +50,7 @@
import io.helidon.common.configurable.ServerThreadPoolSupplier;
import io.helidon.common.http.Http;
import io.helidon.config.Config;
import io.helidon.microprofile.cdi.BuildTimeStart;
import io.helidon.microprofile.cdi.RuntimeStart;
import io.helidon.webserver.Routing;
import io.helidon.webserver.Service;
Expand All @@ -70,6 +72,7 @@ public class ServerCdiExtension implements Extension {
}

private static final Logger LOGGER = Logger.getLogger(ServerCdiExtension.class.getName());
private static final AtomicBoolean IN_PROGRESS_OR_RUNNING = new AtomicBoolean();

// build time
private WebServer.Builder serverBuilder = WebServer.builder()
Expand All @@ -92,6 +95,15 @@ public class ServerCdiExtension implements Extension {
private volatile boolean started;
private final List<JerseySupport> jerseySupports = new LinkedList<>();

private void buildTime(@Observes @BuildTimeStart Object event) {
// update the status of server, as we may have been started without a builder being used
// such as when cdi.Main or SeContainerInitializer are used
if (!IN_PROGRESS_OR_RUNNING.compareAndSet(false, true)) {
throw new IllegalStateException("There is another builder in progress, or another Server running. "
+ "You cannot run more than one in parallel");
}
}

private void prepareRuntime(@Observes @RuntimeStart Config config) {
serverBuilder.config(config.get("server"));
this.config = config;
Expand Down Expand Up @@ -216,6 +228,17 @@ private void registerClasspathStaticContent(Config config) {
}

private void stopServer(@Observes @Priority(PLATFORM_BEFORE) @BeforeDestroyed(ApplicationScoped.class) Object event) {
boolean isRunning = IN_PROGRESS_OR_RUNNING.get();
try {
doStop(event);
} finally {
if (isRunning) {
IN_PROGRESS_OR_RUNNING.set(false);
}
tomas-langer marked this conversation as resolved.
Show resolved Hide resolved
}
}

private void doStop(Object event) {
if (null == webserver) {
// nothing to do
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,10 @@

import io.helidon.microprofile.cdi.HelidonContainer;

import static io.helidon.microprofile.server.Server.Builder.IN_PROGRESS_OR_RUNNING;

/**
* Server to handle lifecycle of microprofile implementation.
*/
public class ServerImpl implements Server {
private static final Logger LOGGER = Logger.getLogger(ServerImpl.class.getName());
private static final Logger STARTUP_LOGGER = Logger.getLogger("io.helidon.microprofile.startup.server");

private final HelidonContainer helidonContainer = HelidonContainer.instance();
Expand Down Expand Up @@ -97,7 +94,6 @@ public Server start() {
@Override
public Server stop() {
container.close();
IN_PROGRESS_OR_RUNNING.set(false);
return this;
}

Expand Down
Loading