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

Correct clients getting shutdown by request scope #10890

Merged
merged 2 commits into from
Jun 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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 @@ -136,6 +136,8 @@ public class ConnectionManager {
private final ClientSslBuilder nettyClientSslBuilder;
private EventLoopGroup group;
private final boolean shutdownGroup;

private final AtomicBoolean running = new AtomicBoolean(false);
private final ThreadFactory threadFactory;
private final ChannelFactory<? extends Channel> socketChannelFactory;
private final ChannelFactory<? extends Channel> udpChannelFactory;
Expand Down Expand Up @@ -206,6 +208,7 @@ public class ConnectionManager {
shutdownGroup = true;
}


refresh();
}

Expand All @@ -224,6 +227,7 @@ final void refresh() {
http3SslContext = null;
}
initBootstrap();
running.set(true);
for (Pool pool : pools.values()) {
pool.forEachConnection(c -> ((Pool.ConnectionHolder) c).windDownConnection());
}
Expand Down Expand Up @@ -315,10 +319,12 @@ final int liveRequestCount() {
* @see DefaultHttpClient#start()
*/
public final void start() {
// only need to start new group if it's managed by us
if (shutdownGroup) {
group = createEventLoopGroup(configuration, threadFactory);
initBootstrap(); // rebuild bootstrap with new group
if (running.compareAndSet(false, true)) {
// only need to start new group if it's managed by us
if (shutdownGroup) {
group = createEventLoopGroup(configuration, threadFactory);
initBootstrap(); // rebuild bootstrap with new group
}
}
}

Expand Down Expand Up @@ -352,31 +358,34 @@ private void initBootstrap() {
* @see DefaultHttpClient#stop()
*/
public final void shutdown() {
for (Pool pool : pools.values()) {
pool.shutdown();
}
if (shutdownGroup) {
Duration shutdownTimeout = configuration.getShutdownTimeout()
.orElse(Duration.ofMillis(HttpClientConfiguration.DEFAULT_SHUTDOWN_TIMEOUT_MILLISECONDS));
Duration shutdownQuietPeriod = configuration.getShutdownQuietPeriod()
.orElse(Duration.ofMillis(HttpClientConfiguration.DEFAULT_SHUTDOWN_QUIET_PERIOD_MILLISECONDS));

Future<?> future = group.shutdownGracefully(
shutdownQuietPeriod.toMillis(),
shutdownTimeout.toMillis(),
TimeUnit.MILLISECONDS
);
try {
future.await(shutdownTimeout.toMillis());
} catch (InterruptedException e) {
// ignore
Thread.currentThread().interrupt();
if (running.compareAndSet(true, false)) {

for (Pool pool : pools.values()) {
pool.shutdown();
}
if (shutdownGroup) {
Duration shutdownTimeout = configuration.getShutdownTimeout()
.orElse(Duration.ofMillis(HttpClientConfiguration.DEFAULT_SHUTDOWN_TIMEOUT_MILLISECONDS));
Duration shutdownQuietPeriod = configuration.getShutdownQuietPeriod()
.orElse(Duration.ofMillis(HttpClientConfiguration.DEFAULT_SHUTDOWN_QUIET_PERIOD_MILLISECONDS));

Future<?> future = group.shutdownGracefully(
shutdownQuietPeriod.toMillis(),
shutdownTimeout.toMillis(),
TimeUnit.MILLISECONDS
);
try {
future.await(shutdownTimeout.toMillis());
} catch (InterruptedException e) {
// ignore
Thread.currentThread().interrupt();
}
}
ReferenceCountUtil.release(sslContext);
ReferenceCountUtil.release(websocketSslContext);
sslContext = null;
websocketSslContext = null;
}
ReferenceCountUtil.release(sslContext);
ReferenceCountUtil.release(websocketSslContext);
sslContext = null;
websocketSslContext = null;
}

/**
Expand All @@ -385,7 +394,7 @@ public final void shutdown() {
* @return Whether this connection manager is still running and can serve requests
*/
public final boolean isRunning() {
return !group.isShutdown();
return running.get() && !group.isShutdown();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,12 @@ import io.micronaut.context.event.ApplicationEventListener
import io.micronaut.http.HttpRequest
import io.micronaut.http.annotation.Controller
import io.micronaut.http.annotation.Get
import io.micronaut.http.client.HttpClient
import io.micronaut.http.client.annotation.Client
import io.micronaut.http.context.event.HttpRequestTerminatedEvent
import io.micronaut.http.server.netty.AbstractMicronautSpec
import io.micronaut.scheduling.TaskExecutors
import io.micronaut.scheduling.annotation.ExecuteOn
import jakarta.annotation.PreDestroy
import jakarta.inject.Inject
import jakarta.inject.Singleton
Expand Down Expand Up @@ -174,13 +178,17 @@ class RequestScopeSpec extends AbstractMicronautSpec {
static class SimpleRequestBean {

private final SimpleBean simpleBean
final HttpClient client

SimpleRequestBean(SimpleBean simpleBean) {
SimpleRequestBean(
SimpleBean simpleBean,
@Client("/") HttpClient client) {
this.simpleBean = simpleBean
this.client = client
}

String sayHello() {
return "HELLO"
return client.toBlocking().retrieve("/test-simple-request-scope-other")
}

}
Expand All @@ -200,15 +208,27 @@ class RequestScopeSpec extends AbstractMicronautSpec {
@Controller
static class SimpleTestController {
final SimpleRequestBean simpleRequestBean
final HttpClient client

SimpleTestController(SimpleRequestBean simpleRequestBean) {
SimpleTestController(
SimpleRequestBean simpleRequestBean,
@Client("/") HttpClient client) {
this.simpleRequestBean = simpleRequestBean
this.client = client
}

@Get("/test-simple-request-scope")
@ExecuteOn(TaskExecutors.BLOCKING)
String test() {
return simpleRequestBean.sayHello()
}

@Get("/test-simple-request-scope-other")
String test2() {
simpleRequestBean.client.is(client)
assert simpleRequestBean.client.isRunning()
return "HELLO"
}
}

@RequestScope
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1204,7 +1204,7 @@ private <T> void destroyBean(@NonNull BeanRegistration<T> registration, boolean
}
}
}
if (beanToDestroy instanceof LifeCycle<?> cycle) {
if (beanToDestroy instanceof LifeCycle<?> cycle && !dependent) {
destroyLifeCycleBean(cycle, definition);
}
if (registration instanceof BeanDisposingRegistration) {
Expand Down
Loading