You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by hq...@apache.org on 2026/10/08 04:02:55 UTC
(james-project) branch master updated (ee658a741f -> 12b00468d1)
This is an automated email from the ASF dual-hosted git repository.
quantranhong1999 pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
from ee658a741f [SITE] Remove 3.7.x from the download page (#3235)
new 5a394e1a99 [FIX] Lifecycle: run @PreDestroy of provider-built singletons
new 12b00468d1 [FIX] KeyRegistrationHandler: bound the event bus stop
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../james/events/KeyRegistrationHandler.java | 13 +-
.../apache/james/events/RabbitMQEventBusTest.java | 9 +
.../lifecycle/AbstractMethodTypeListener.java | 54 ++--
.../onami/lifecycle/LifeCycleStageModule.java | 77 ++++-
.../ProviderBuiltSingletonStagingTest.java | 338 +++++++++++++++++++++
.../james/modules/protocols/ImapGuiceProbe.java | 6 -
.../james/modules/protocols/SmtpGuiceProbe.java | 7 -
7 files changed, 457 insertions(+), 47 deletions(-)
create mode 100644 server/container/guice/onami/src/test/java/org/apache/james/onami/lifecycle/ProviderBuiltSingletonStagingTest.java
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
(james-project) 01/02: [FIX] Lifecycle: run @PreDestroy of provider-built singletons
Posted by hq...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
quantranhong1999 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 5a394e1a996bfaf43ceb5a202bcec04f18da82f8
Author: Quan Tran <hq...@linagora.com>
AuthorDate: Sat Oct 3 10:15:19 2026 +0700
[FIX] Lifecycle: run @PreDestroy of provider-built singletons
@PreDestroy methods were only registered for objects Guice constructs or
injects itself. Singletons returned by providers (@Provides methods,
provider instances or classes) were never stopped: RabbitMQ event buses,
channel pools, the Cassandra session...
They are now stopped by GuiceJamesServer.stop(), so a production shutdown
releases them, and restarting servers in one JVM no longer leaks ~160
threads per restart, which lets tests reuse surefire forks.
The SmtpGuiceProbe and ImapGuiceProbe workarounds destroying their server
factories by hand are removed.
Co-Authored-By: Claude Opus 5.5 <no...@anthropic.com>
---
.../lifecycle/AbstractMethodTypeListener.java | 54 ++--
.../onami/lifecycle/LifeCycleStageModule.java | 77 ++++-
.../ProviderBuiltSingletonStagingTest.java | 338 +++++++++++++++++++++
.../james/modules/protocols/ImapGuiceProbe.java | 6 -
.../james/modules/protocols/SmtpGuiceProbe.java | 7 -
5 files changed, 439 insertions(+), 43 deletions(-)
diff --git a/server/container/guice/onami/src/main/java/org/apache/james/onami/lifecycle/AbstractMethodTypeListener.java b/server/container/guice/onami/src/main/java/org/apache/james/onami/lifecycle/AbstractMethodTypeListener.java
index aae0bbbc04..2b49380777 100644
--- a/server/container/guice/onami/src/main/java/org/apache/james/onami/lifecycle/AbstractMethodTypeListener.java
+++ b/server/container/guice/onami/src/main/java/org/apache/james/onami/lifecycle/AbstractMethodTypeListener.java
@@ -21,6 +21,7 @@ package org.apache.james.onami.lifecycle;
import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
+import java.util.ArrayList;
import java.util.List;
import com.google.inject.TypeLiteral;
@@ -37,6 +38,25 @@ abstract class AbstractMethodTypeListener implements TypeListener {
*/
private static final String JAVA_PACKAGE = "java";
+ /**
+ * Lists the input klass and its superclasses, stopping at the first {@code java} package class.
+ *
+ * @param klass the class to start from.
+ * @return the classes to scan for lifecycle methods, from the input klass up.
+ */
+ static List<Class<?>> lifecycleHierarchy(Class<?> klass) {
+ List<Class<?>> hierarchy = new ArrayList<>();
+ for (Class<?> current = klass; current != null && !isJavaClass(current); current = current.getSuperclass()) {
+ hierarchy.add(current);
+ }
+ return hierarchy;
+ }
+
+ private static boolean isJavaClass(Class<?> klass) {
+ Package pkg = klass.getPackage();
+ return pkg != null && pkg.getName().startsWith(JAVA_PACKAGE);
+ }
+
/**
* The lifecycle annotations to search on methods in the order to be searched.
*/
@@ -53,36 +73,20 @@ abstract class AbstractMethodTypeListener implements TypeListener {
@Override
public final <I> void hear(TypeLiteral<I> type, TypeEncounter<I> encounter) {
- hear(type, type.getRawType(), encounter);
- }
-
- /**
- * Allows traverse the input klass hierarchy.
- *
- * @param parentType the owning type being heard
- * @param klass encountered by Guice.
- * @param encounter the injection context.
- */
- private <I> void hear(final TypeLiteral<I> parentType, Class<? super I> klass, TypeEncounter<I> encounter) {
- Package pkg;
- if (klass == null || ((pkg = klass.getPackage()) != null && pkg.getName().startsWith(JAVA_PACKAGE))) {
- return;
- }
+ for (Class<?> klass : lifecycleHierarchy(type.getRawType())) {
+ for (Class<? extends Annotation> annotationType : annotationTypes) {
+ for (Method method : klass.getDeclaredMethods()) {
+ if (method.isAnnotationPresent(annotationType)) {
+ if (method.getParameterTypes().length != 0) {
+ encounter.addError("Annotated methods with @%s must not accept any argument, found %s",
+ annotationType.getName(), method);
+ }
- for (Class<? extends Annotation> annotationType : annotationTypes) {
- for (Method method : klass.getDeclaredMethods()) {
- if (method.isAnnotationPresent(annotationType)) {
- if (method.getParameterTypes().length != 0) {
- encounter.addError("Annotated methods with @%s must not accept any argument, found %s",
- annotationType.getName(), method);
+ hear(method, type, encounter, annotationType);
}
-
- hear(method, parentType, encounter, annotationType);
}
}
}
-
- hear(parentType, klass.getSuperclass(), encounter);
}
/**
diff --git a/server/container/guice/onami/src/main/java/org/apache/james/onami/lifecycle/LifeCycleStageModule.java b/server/container/guice/onami/src/main/java/org/apache/james/onami/lifecycle/LifeCycleStageModule.java
index a5b3d5f799..2f13a3f19a 100644
--- a/server/container/guice/onami/src/main/java/org/apache/james/onami/lifecycle/LifeCycleStageModule.java
+++ b/server/container/guice/onami/src/main/java/org/apache/james/onami/lifecycle/LifeCycleStageModule.java
@@ -24,14 +24,26 @@ import static java.util.Arrays.asList;
import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
import java.lang.reflect.ParameterizedType;
import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import com.google.inject.Binding;
import com.google.inject.Key;
+import com.google.inject.Scopes;
import com.google.inject.TypeLiteral;
+import com.google.inject.matcher.AbstractMatcher;
import com.google.inject.matcher.Matcher;
import com.google.inject.spi.InjectionListener;
+import com.google.inject.spi.ProviderInstanceBinding;
+import com.google.inject.spi.ProviderKeyBinding;
+import com.google.inject.spi.ProvisionListener;
import com.google.inject.spi.TypeEncounter;
import com.google.inject.util.Types;
@@ -68,6 +80,41 @@ public abstract class LifeCycleStageModule extends LifeCycleModule {
return stagerType;
}
+ private static boolean isProviderBuiltSingleton(Binding<?> binding) {
+ return (binding instanceof ProviderInstanceBinding || binding instanceof ProviderKeyBinding) && Scopes.isSingleton(binding);
+ }
+
+ private static List<Method> stageMethods(Class<?> klass, Class<? extends Annotation> stage) {
+ return AbstractMethodTypeListener.lifecycleHierarchy(klass).stream()
+ .flatMap(type -> Arrays.stream(type.getDeclaredMethods()))
+ .filter(method -> method.isAnnotationPresent(stage) && method.getParameterCount() == 0)
+ .toList();
+ }
+
+ // Invoking an overridden declaration runs the override: both must be registered only once
+ private static Object overrideKey(Method method) {
+ if (Modifier.isPrivate(method.getModifiers())) {
+ return method;
+ }
+ return List.of(method.getName(), List.of(method.getParameterTypes()));
+ }
+
+ private static boolean isFirstRegistration(Map<Object, Set<Object>> registeredMethods, Object instance, Method stageMethod) {
+ synchronized (registeredMethods) {
+ return registeredMethods.computeIfAbsent(instance, any -> new HashSet<>())
+ .add(overrideKey(stageMethod));
+ }
+ }
+
+ private static void registerOnce(Map<Object, Set<Object>> registeredMethods, Stager<?> stager, StageableTypeMapper typeMapper,
+ Object instance, Method stageMethod, TypeLiteral<?> type) {
+ if (isFirstRegistration(registeredMethods, instance, stageMethod)) {
+ Stageable stageable = new StageableMethod(stageMethod, instance);
+ stager.register(stageable);
+ typeMapper.registerType(stageable, type);
+ }
+ }
+
@Override
protected final void configure() {
if (bindings != null) {
@@ -87,17 +134,37 @@ public abstract class LifeCycleStageModule extends LifeCycleModule {
private <A extends Annotation> void bind(BindingBuilder<A> binding) {
final Stager<A> stager = binding.stager;
final StageableTypeMapper typeMapper = binding.typeMapper;
+ // Shared by both listeners: an instance both injected and returned by a provider, or exposed under several keys, is staged once
+ final Map<Object, Set<Object>> registeredMethods = new IdentityHashMap<>();
bind(type(stager.getStage())).toInstance(stager);
bindListener(binding.typeMatcher, new AbstractMethodTypeListener(asList(stager.getStage())) {
@Override
protected <I> void hear(final Method stageMethod, final TypeLiteral<I> parentType,
final TypeEncounter<I> encounter, final Class<? extends Annotation> annotationType) {
- encounter.register((InjectionListener<I>) injectee -> {
- Stageable stageable = new StageableMethod(stageMethod, injectee);
- stager.register(stageable);
- typeMapper.registerType(stageable, parentType);
- });
+ encounter.register((InjectionListener<I>) injectee ->
+ registerOnce(registeredMethods, stager, typeMapper, injectee, stageMethod, parentType));
+ }
+ });
+
+ // Guice does not inject objects returned by providers (provider methods, instances or classes), so the type listener above never hears them
+ bindListener(new AbstractMatcher<Binding<?>>() {
+ @Override
+ public boolean matches(Binding<?> candidate) {
+ return isProviderBuiltSingleton(candidate);
+ }
+ }, new ProvisionListener() {
+ @Override
+ public <T> void onProvision(ProvisionInvocation<T> provision) {
+ T instance = provision.provision();
+ if (instance == null) {
+ return;
+ }
+ TypeLiteral<?> instanceType = TypeLiteral.get(instance.getClass());
+ if (binding.typeMatcher.matches(instanceType)) {
+ stageMethods(instance.getClass(), stager.getStage())
+ .forEach(stageMethod -> registerOnce(registeredMethods, stager, typeMapper, instance, stageMethod, instanceType));
+ }
}
});
}
diff --git a/server/container/guice/onami/src/test/java/org/apache/james/onami/lifecycle/ProviderBuiltSingletonStagingTest.java b/server/container/guice/onami/src/test/java/org/apache/james/onami/lifecycle/ProviderBuiltSingletonStagingTest.java
new file mode 100644
index 0000000000..a28c82e32a
--- /dev/null
+++ b/server/container/guice/onami/src/test/java/org/apache/james/onami/lifecycle/ProviderBuiltSingletonStagingTest.java
@@ -0,0 +1,338 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you 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 org.apache.james.onami.lifecycle;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import jakarta.annotation.PreDestroy;
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.inject.Singleton;
+
+import org.junit.jupiter.api.Test;
+
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Module;
+import com.google.inject.Provides;
+import com.google.inject.Scopes;
+import com.google.inject.TypeLiteral;
+
+class ProviderBuiltSingletonStagingTest {
+ interface Stoppable {
+ }
+
+ static class StopRecorder implements Stoppable {
+ private final String name;
+ private final List<String> stops;
+
+ StopRecorder(String name, List<String> stops) {
+ this.name = name;
+ this.stops = stops;
+ }
+
+ @PreDestroy
+ void stop() {
+ stops.add(name);
+ }
+ }
+
+ static class InheritingStopRecorder extends StopRecorder {
+ InheritingStopRecorder(String name, List<String> stops) {
+ super(name, stops);
+ }
+ }
+
+ static class InjectedStopRecorder extends StopRecorder {
+ @Inject
+ InjectedStopRecorder(List<String> stops) {
+ super("injected", stops);
+ }
+ }
+
+ static class OverridingStopRecorder extends StopRecorder {
+ OverridingStopRecorder(String name, List<String> stops) {
+ super(name, stops);
+ }
+
+ @Override
+ @PreDestroy
+ void stop() {
+ super.stop();
+ }
+ }
+
+ static class InjectedOverridingStopRecorder extends StopRecorder {
+ @Inject
+ InjectedOverridingStopRecorder(List<String> stops) {
+ super("injected-overriding", stops);
+ }
+
+ @Override
+ @PreDestroy
+ void stop() {
+ super.stop();
+ }
+ }
+
+ static class PrivateStopBase {
+ final List<String> stops;
+
+ PrivateStopBase(List<String> stops) {
+ this.stops = stops;
+ }
+
+ @PreDestroy
+ private void stop() {
+ stops.add("base");
+ }
+ }
+
+ static class PrivateStopDerived extends PrivateStopBase {
+ PrivateStopDerived(List<String> stops) {
+ super(stops);
+ }
+
+ @PreDestroy
+ private void stop() {
+ stops.add("derived");
+ }
+ }
+
+ static class StopRecorderProvider implements Provider<Stoppable> {
+ private final List<String> stops;
+
+ @Inject
+ StopRecorderProvider(List<String> stops) {
+ this.stops = stops;
+ }
+
+ @Override
+ public Stoppable get() {
+ return new StopRecorder("provider-class", stops);
+ }
+ }
+
+ private final List<String> stops = new ArrayList<>();
+ private final PreDestroyModule preDestroyModule = new PreDestroyModule();
+
+ private Injector injector(Module module) {
+ return Guice.createInjector(preDestroyModule, binder -> binder.bind(new TypeLiteral<List<String>>() { }).toInstance(stops), module);
+ }
+
+ @Test
+ void preDestroyOfProviderBuiltSingletonShouldBeCalledOnce() {
+ Injector injector = injector(new AbstractModule() {
+ @Provides
+ @Singleton
+ StopRecorder stopRecorder() {
+ return new StopRecorder("provided", stops);
+ }
+ });
+ injector.getInstance(StopRecorder.class);
+ injector.getInstance(StopRecorder.class);
+
+ preDestroyModule.getStager().stage();
+
+ assertThat(stops).containsExactly("provided");
+ }
+
+ @Test
+ void inheritedPreDestroyOfProviderBuiltSingletonShouldBeCalled() {
+ Injector injector = injector(new AbstractModule() {
+ @Provides
+ @Singleton
+ StopRecorder stopRecorder() {
+ return new InheritingStopRecorder("inheriting", stops);
+ }
+ });
+ injector.getInstance(StopRecorder.class);
+
+ preDestroyModule.getStager().stage();
+
+ assertThat(stops).containsExactly("inheriting");
+ }
+
+ @Test
+ void preDestroyOfConstructorInjectedSingletonShouldBeCalledOnce() {
+ Injector injector = injector(binder -> binder.bind(InjectedStopRecorder.class).in(Scopes.SINGLETON));
+ injector.getInstance(InjectedStopRecorder.class);
+ injector.getInstance(InjectedStopRecorder.class);
+
+ preDestroyModule.getStager().stage();
+
+ assertThat(stops).containsExactly("injected");
+ }
+
+ @Test
+ void preDestroyOfInjectedSingletonReturnedByProviderShouldBeCalledOnce() {
+ Injector injector = injector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(InjectedStopRecorder.class).in(Scopes.SINGLETON);
+ }
+
+ @Provides
+ @Singleton
+ Stoppable stoppable(InjectedStopRecorder stopRecorder) {
+ return stopRecorder;
+ }
+ });
+ injector.getInstance(Stoppable.class);
+ injector.getInstance(InjectedStopRecorder.class);
+
+ preDestroyModule.getStager().stage();
+
+ assertThat(stops).containsExactly("injected");
+ }
+
+ @Test
+ void preDestroyOfProviderBuiltSingletonExposedUnderTwoKeysShouldBeCalledOnce() {
+ Injector injector = injector(new AbstractModule() {
+ @Provides
+ @Singleton
+ StopRecorder stopRecorder() {
+ return new StopRecorder("provided", stops);
+ }
+
+ @Provides
+ @Singleton
+ Stoppable stoppable(StopRecorder stopRecorder) {
+ return stopRecorder;
+ }
+ });
+ injector.getInstance(Stoppable.class);
+ injector.getInstance(StopRecorder.class);
+
+ preDestroyModule.getStager().stage();
+
+ assertThat(stops).containsExactly("provided");
+ }
+
+ @Test
+ void providerBuiltSingletonShouldBeStoppedBeforeItsDependencies() {
+ Injector injector = injector(new AbstractModule() {
+ @Provides
+ @Singleton
+ StopRecorder dependency() {
+ return new StopRecorder("dependency", stops);
+ }
+
+ @Provides
+ @Singleton
+ Stoppable dependent(StopRecorder dependency) {
+ return new StopRecorder("dependent", stops);
+ }
+ });
+ injector.getInstance(Stoppable.class);
+
+ preDestroyModule.getStager().stage();
+
+ assertThat(stops).containsExactly("dependent", "dependency");
+ }
+
+ @Test
+ void preDestroyOfUnscopedProviderBuiltObjectShouldNotBeCalled() {
+ Injector injector = injector(new AbstractModule() {
+ @Provides
+ StopRecorder stopRecorder() {
+ return new StopRecorder("unscoped", stops);
+ }
+ });
+ injector.getInstance(StopRecorder.class);
+
+ preDestroyModule.getStager().stage();
+
+ assertThat(stops).isEmpty();
+ }
+
+ @Test
+ void preDestroyOfProviderBuiltSingletonInjectedLaterShouldBeCalledOnce() {
+ Injector injector = injector(new AbstractModule() {
+ @Provides
+ @Singleton
+ StopRecorder stopRecorder() {
+ return new StopRecorder("provided", stops);
+ }
+ });
+ injector.injectMembers(injector.getInstance(StopRecorder.class));
+
+ preDestroyModule.getStager().stage();
+
+ assertThat(stops).containsExactly("provided");
+ }
+
+ @Test
+ void overriddenPreDestroyOfProviderBuiltSingletonShouldBeCalledOnce() {
+ Injector injector = injector(new AbstractModule() {
+ @Provides
+ @Singleton
+ StopRecorder stopRecorder() {
+ return new OverridingStopRecorder("overriding", stops);
+ }
+ });
+ injector.getInstance(StopRecorder.class);
+
+ preDestroyModule.getStager().stage();
+
+ assertThat(stops).containsExactly("overriding");
+ }
+
+ @Test
+ void overriddenPreDestroyOfConstructorInjectedSingletonShouldBeCalledOnce() {
+ Injector injector = injector(binder -> binder.bind(InjectedOverridingStopRecorder.class).in(Scopes.SINGLETON));
+ injector.getInstance(InjectedOverridingStopRecorder.class);
+
+ preDestroyModule.getStager().stage();
+
+ assertThat(stops).containsExactly("injected-overriding");
+ }
+
+ @Test
+ void privatePreDestroyMethodsOfSubclassAndSuperclassShouldBothBeCalled() {
+ Injector injector = injector(new AbstractModule() {
+ @Provides
+ @Singleton
+ PrivateStopBase privateStop() {
+ return new PrivateStopDerived(stops);
+ }
+ });
+ injector.getInstance(PrivateStopBase.class);
+
+ preDestroyModule.getStager().stage();
+
+ assertThat(stops).containsExactlyInAnyOrder("derived", "base");
+ }
+
+ @Test
+ void preDestroyOfSingletonBuiltByProviderClassShouldBeCalledOnce() {
+ Injector injector = injector(binder -> binder.bind(Stoppable.class).toProvider(StopRecorderProvider.class).in(Scopes.SINGLETON));
+ injector.getInstance(Stoppable.class);
+ injector.getInstance(Stoppable.class);
+
+ preDestroyModule.getStager().stage();
+
+ assertThat(stops).containsExactly("provider-class");
+ }
+}
diff --git a/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/ImapGuiceProbe.java b/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/ImapGuiceProbe.java
index a216fbfae4..1c2165c2df 100644
--- a/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/ImapGuiceProbe.java
+++ b/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/ImapGuiceProbe.java
@@ -22,7 +22,6 @@ import java.net.InetSocketAddress;
import java.util.Optional;
import java.util.function.Predicate;
-import jakarta.annotation.PreDestroy;
import jakarta.inject.Inject;
import org.apache.james.imapserver.netty.IMAPServerFactory;
@@ -39,11 +38,6 @@ public class ImapGuiceProbe implements GuiceProbe {
this.imapServerFactory = imapServerFactory;
}
- @PreDestroy
- void destroy() {
- imapServerFactory.destroy();
- }
-
public int getImapPort() {
return getPort(Predicate.not(AbstractConfigurableAsyncServer::getStartTLSSupported))
.orElseThrow(() -> new IllegalStateException("IMAP server not defined"));
diff --git a/server/container/guice/protocols/smtp/src/main/java/org/apache/james/modules/protocols/SmtpGuiceProbe.java b/server/container/guice/protocols/smtp/src/main/java/org/apache/james/modules/protocols/SmtpGuiceProbe.java
index e96153afbc..9c67ced130 100644
--- a/server/container/guice/protocols/smtp/src/main/java/org/apache/james/modules/protocols/SmtpGuiceProbe.java
+++ b/server/container/guice/protocols/smtp/src/main/java/org/apache/james/modules/protocols/SmtpGuiceProbe.java
@@ -24,7 +24,6 @@ import java.net.InetSocketAddress;
import java.util.function.Function;
import java.util.function.Predicate;
-import jakarta.annotation.PreDestroy;
import jakarta.inject.Inject;
import org.apache.james.protocols.lib.netty.AbstractConfigurableAsyncServer;
@@ -57,12 +56,6 @@ public class SmtpGuiceProbe implements GuiceProbe {
this.smtpServerFactory = smtpServerFactory;
}
- @PreDestroy
- void destroy() {
- // SMTPServerFactory is provided through a factory method; dispose it explicitly on Guice shutdown.
- smtpServerFactory.destroy();
- }
-
public Port getSmtpPort() {
return getPort(server -> true);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
(james-project) 02/02: [FIX] KeyRegistrationHandler: bound the event bus stop
Posted by hq...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
quantranhong1999 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 12b00468d12005e7070b7d9b3ddd78b9d09808d5
Author: Quan Tran <hq...@linagora.com>
AuthorDate: Wed Oct 7 15:10:30 2026 +0700
[FIX] KeyRegistrationHandler: bound the event bus stop
Stopping a RabbitMQ event bus deleted its key registration queue first,
with a one minute timeout retried up to 8 times, and only then disposed the
registration consumer and its scheduler. With an unresponsive broker one
bus could block stop for about ten minutes, then skip the local cleanup.
Each bus is stopped in turn, and these stops now run on every server
shutdown since provider-built singletons get their @PreDestroy called.
The consumer and scheduler are now disposed first. Deleting the queue is a
single best effort attempt bounded to 10 seconds; a failure is logged
instead of thrown. The registration queue is auto-delete by default with
classic queues; a queue left behind otherwise is the same leftover a crash
already leaves.
New test: stopping the event bus while RabbitMQ is paused returns within
30 seconds (it timed out before). RabbitMQEventBusTest: 81 tests green.
Co-Authored-By: Claude Opus 5.5 <no...@anthropic.com>
---
.../org/apache/james/events/KeyRegistrationHandler.java | 13 +++++++++----
.../java/org/apache/james/events/RabbitMQEventBusTest.java | 9 +++++++++
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/event-bus/distributed/src/main/java/org/apache/james/events/KeyRegistrationHandler.java b/event-bus/distributed/src/main/java/org/apache/james/events/KeyRegistrationHandler.java
index b1a53a4444..2dc2f2d5ab 100644
--- a/event-bus/distributed/src/main/java/org/apache/james/events/KeyRegistrationHandler.java
+++ b/event-bus/distributed/src/main/java/org/apache/james/events/KeyRegistrationHandler.java
@@ -59,6 +59,7 @@ class KeyRegistrationHandler {
private static final Logger LOGGER = LoggerFactory.getLogger(KeyRegistrationHandler.class);
private static final Duration TOPOLOGY_CHANGES_TIMEOUT = Duration.ofMinutes(1);
+ private static final Duration STOP_QUEUE_DELETION_TIMEOUT = Duration.ofSeconds(10);
private final EventBusId eventBusId;
private final LocalListenerRegistry localListenerRegistry;
@@ -135,13 +136,17 @@ class KeyRegistrationHandler {
}
void stop() {
- sender.delete(QueueSpecification.queue(registrationQueue.asString()))
- .timeout(TOPOLOGY_CHANGES_TIMEOUT)
- .retryWhen(configurations.retryBackoff().asReactorRetry().scheduler(Schedulers.parallel()))
- .block();
receiverSubscriber.filter(Predicate.not(Disposable::isDisposed))
.ifPresent(Disposable::dispose);
Optional.ofNullable(scheduler).ifPresent(Scheduler::dispose);
+ // Best effort and bounded: stopping must not wait minutes on an unavailable broker
+ sender.delete(QueueSpecification.queue(registrationQueue.asString()))
+ .timeout(STOP_QUEUE_DELETION_TIMEOUT)
+ .onErrorResume(e -> {
+ LOGGER.warn("Could not delete key registration queue {} while stopping the event bus", registrationQueue.asString(), e);
+ return Mono.empty();
+ })
+ .block();
}
Mono<Registration> register(EventListener.ReactiveEventListener listener, RegistrationKey key) {
diff --git a/event-bus/distributed/src/test/java/org/apache/james/events/RabbitMQEventBusTest.java b/event-bus/distributed/src/test/java/org/apache/james/events/RabbitMQEventBusTest.java
index ce12c5d628..52bcc47b18 100644
--- a/event-bus/distributed/src/test/java/org/apache/james/events/RabbitMQEventBusTest.java
+++ b/event-bus/distributed/src/test/java/org/apache/james/events/RabbitMQEventBusTest.java
@@ -42,6 +42,7 @@ import static org.awaitility.Awaitility.await;
import static org.awaitility.Durations.FIVE_SECONDS;
import static org.awaitility.Durations.TEN_MINUTES;
import static org.awaitility.Durations.TEN_SECONDS;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
@@ -766,6 +767,14 @@ class RabbitMQEventBusTest implements GroupContract.SingleEventBusGroupContract,
.anySatisfy(queue -> assertThat(queue.getName()).contains(GroupA.class.getName()));
}
+ @Test
+ void stopShouldNotWaitLongWhenRabbitMQIsUnavailable() {
+ eventBus.start();
+ rabbitMQExtension.getRabbitMQ().pause();
+
+ assertTimeoutPreemptively(Duration.ofSeconds(30), () -> eventBus.stop());
+ }
+
@Test
void eventBusShouldNotThrowWhenContinuouslyStartAndStop() {
assertThatCode(() -> {
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org