This is an automated email from the ASF dual-hosted git repository. reta pushed a commit to branch 4.1.x-fixes in repository https://gitbox.apache.org/repos/asf/cxf.git
commit e81af68371d2de5ffcb921d14cb2aaef45ca8ab0 Author: Andriy Redko <[email protected]> AuthorDate: Thu Sep 17 09:23:14 2026 -0400 CXF-9245: Race Condition in ServerLifeCycleManagerImpl#stopServer() (#3475) * CXF-9245: Race Condition in ServerLifeCycleManagerImpl#stopServer() * Address code review comments * Address code review comments Signed-off-by: Andriy Redko <[email protected]> --------- Signed-off-by: Andriy Redko <[email protected]> (cherry picked from commit 3265a9bb6ff57b7d376fecc264f900818a03db6a) --- .../cxf/bus/managers/AbstractLifeCycleManager.java | 41 +++++++ .../cxf/bus/managers/CXFBusLifeCycleManager.java | 13 +-- .../bus/managers/ClientLifeCycleManagerImpl.java | 12 +- .../bus/managers/ServerLifeCycleManagerImpl.java | 13 +-- .../managers/ServerLifeCycleManagerImplTest.java | 124 +++++++++++++++++++++ 5 files changed, 182 insertions(+), 21 deletions(-) diff --git a/core/src/main/java/org/apache/cxf/bus/managers/AbstractLifeCycleManager.java b/core/src/main/java/org/apache/cxf/bus/managers/AbstractLifeCycleManager.java new file mode 100644 index 00000000000..c1c06c7e53e --- /dev/null +++ b/core/src/main/java/org/apache/cxf/bus/managers/AbstractLifeCycleManager.java @@ -0,0 +1,41 @@ +/** + * 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.cxf.bus.managers; + +import java.util.ListIterator; +import java.util.concurrent.CopyOnWriteArrayList; + +/** + * AbstractLifeCycleManager implementation + * @param <T> listener class + */ +abstract class AbstractLifeCycleManager<T> { + protected final CopyOnWriteArrayList<T> listeners; + + AbstractLifeCycleManager() { + listeners = new CopyOnWriteArrayList<>(); + } + + protected ListIterator<T> reversedListIterator() { + @SuppressWarnings("unchecked") + final CopyOnWriteArrayList<T> cloned = (CopyOnWriteArrayList<T>) listeners.clone(); + return cloned.listIterator(cloned.size()); + } +} diff --git a/core/src/main/java/org/apache/cxf/bus/managers/CXFBusLifeCycleManager.java b/core/src/main/java/org/apache/cxf/bus/managers/CXFBusLifeCycleManager.java index 5da5ef79fcd..b287f18f09d 100644 --- a/core/src/main/java/org/apache/cxf/bus/managers/CXFBusLifeCycleManager.java +++ b/core/src/main/java/org/apache/cxf/bus/managers/CXFBusLifeCycleManager.java @@ -20,7 +20,6 @@ package org.apache.cxf.bus.managers; import java.util.ListIterator; -import java.util.concurrent.CopyOnWriteArrayList; import jakarta.annotation.Resource; import org.apache.cxf.Bus; @@ -30,19 +29,19 @@ import org.apache.cxf.common.injection.NoJSR250Annotations; import org.apache.cxf.configuration.ConfiguredBeanLocator; @NoJSR250Annotations(unlessNull = "bus") -public class CXFBusLifeCycleManager implements BusLifeCycleManager { +public class CXFBusLifeCycleManager extends AbstractLifeCycleManager<BusLifeCycleListener> + implements BusLifeCycleManager { - private final CopyOnWriteArrayList<BusLifeCycleListener> listeners; private Bus bus; private boolean initCalled; private boolean preShutdownCalled; private boolean postShutdownCalled; public CXFBusLifeCycleManager() { - listeners = new CopyOnWriteArrayList<>(); + super(); } public CXFBusLifeCycleManager(Bus b) { - listeners = new CopyOnWriteArrayList<>(); + super(); setBus(b); } @@ -91,7 +90,7 @@ public class CXFBusLifeCycleManager implements BusLifeCycleManager { public void preShutdown() { if (!preShutdownCalled) { preShutdownCalled = true; - ListIterator<BusLifeCycleListener> li = listeners.listIterator(listeners.size()); + ListIterator<BusLifeCycleListener> li = reversedListIterator(); while (li.hasPrevious()) { li.previous().preShutdown(); } @@ -104,7 +103,7 @@ public class CXFBusLifeCycleManager implements BusLifeCycleManager { } if (!postShutdownCalled) { postShutdownCalled = true; - ListIterator<BusLifeCycleListener> li = listeners.listIterator(listeners.size()); + ListIterator<BusLifeCycleListener> li = reversedListIterator(); while (li.hasPrevious()) { li.previous().postShutdown(); } diff --git a/core/src/main/java/org/apache/cxf/bus/managers/ClientLifeCycleManagerImpl.java b/core/src/main/java/org/apache/cxf/bus/managers/ClientLifeCycleManagerImpl.java index bf870de288d..7b64997b587 100644 --- a/core/src/main/java/org/apache/cxf/bus/managers/ClientLifeCycleManagerImpl.java +++ b/core/src/main/java/org/apache/cxf/bus/managers/ClientLifeCycleManagerImpl.java @@ -21,7 +21,6 @@ package org.apache.cxf.bus.managers; import java.util.Collection; import java.util.ListIterator; -import java.util.concurrent.CopyOnWriteArrayList; import org.apache.cxf.Bus; import org.apache.cxf.common.injection.NoJSR250Annotations; @@ -32,16 +31,15 @@ import org.apache.cxf.endpoint.ClientLifeCycleManager; import org.apache.cxf.extension.BusExtension; @NoJSR250Annotations -public class ClientLifeCycleManagerImpl implements ClientLifeCycleManager, BusExtension { - - private CopyOnWriteArrayList<ClientLifeCycleListener> listeners - = new CopyOnWriteArrayList<>(); +public class ClientLifeCycleManagerImpl extends AbstractLifeCycleManager<ClientLifeCycleListener> + implements ClientLifeCycleManager, BusExtension { public ClientLifeCycleManagerImpl() { - + super(); } public ClientLifeCycleManagerImpl(Bus b) { + super(); Collection<? extends ClientLifeCycleListener> l = b.getExtension(ConfiguredBeanLocator.class) .getBeansOfType(ClientLifeCycleListener.class); if (l != null) { @@ -64,7 +62,7 @@ public class ClientLifeCycleManagerImpl implements ClientLifeCycleManager, BusEx } public void clientDestroyed(Client client) { - ListIterator<ClientLifeCycleListener> li = listeners.listIterator(listeners.size()); + ListIterator<ClientLifeCycleListener> li = reversedListIterator(); while (li.hasPrevious()) { li.previous().clientDestroyed(client); } diff --git a/core/src/main/java/org/apache/cxf/bus/managers/ServerLifeCycleManagerImpl.java b/core/src/main/java/org/apache/cxf/bus/managers/ServerLifeCycleManagerImpl.java index a129dad3083..4e52c25ce1d 100644 --- a/core/src/main/java/org/apache/cxf/bus/managers/ServerLifeCycleManagerImpl.java +++ b/core/src/main/java/org/apache/cxf/bus/managers/ServerLifeCycleManagerImpl.java @@ -21,7 +21,6 @@ package org.apache.cxf.bus.managers; import java.util.Collection; import java.util.ListIterator; -import java.util.concurrent.CopyOnWriteArrayList; import org.apache.cxf.Bus; import org.apache.cxf.common.injection.NoJSR250Annotations; @@ -32,15 +31,15 @@ import org.apache.cxf.endpoint.ServerLifeCycleManager; import org.apache.cxf.extension.BusExtension; @NoJSR250Annotations -public class ServerLifeCycleManagerImpl implements ServerLifeCycleManager, BusExtension { - - private CopyOnWriteArrayList<ServerLifeCycleListener> listeners = - new CopyOnWriteArrayList<>(); +public class ServerLifeCycleManagerImpl extends AbstractLifeCycleManager<ServerLifeCycleListener> + implements ServerLifeCycleManager, BusExtension { public ServerLifeCycleManagerImpl() { - + super(); } + public ServerLifeCycleManagerImpl(Bus b) { + super(); Collection<? extends ServerLifeCycleListener> l = b.getExtension(ConfiguredBeanLocator.class) .getBeansOfType(ServerLifeCycleListener.class); if (l != null) { @@ -63,7 +62,7 @@ public class ServerLifeCycleManagerImpl implements ServerLifeCycleManager, BusEx } public void stopServer(Server server) { - ListIterator<ServerLifeCycleListener> li = listeners.listIterator(listeners.size()); + ListIterator<ServerLifeCycleListener> li = reversedListIterator(); while (li.hasPrevious()) { li.previous().stopServer(server); } diff --git a/core/src/test/java/org/apache/cxf/bus/managers/ServerLifeCycleManagerImplTest.java b/core/src/test/java/org/apache/cxf/bus/managers/ServerLifeCycleManagerImplTest.java new file mode 100644 index 00000000000..fa3e7dee9c3 --- /dev/null +++ b/core/src/test/java/org/apache/cxf/bus/managers/ServerLifeCycleManagerImplTest.java @@ -0,0 +1,124 @@ +/** + * 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.cxf.bus.managers; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Random; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.Phaser; +import java.util.concurrent.TimeUnit; + +import org.apache.cxf.endpoint.Endpoint; +import org.apache.cxf.endpoint.Server; +import org.apache.cxf.endpoint.ServerLifeCycleListener; +import org.apache.cxf.endpoint.ServerLifeCycleManager; +import org.apache.cxf.transport.Destination; + +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; + +public class ServerLifeCycleManagerImplTest { + private static final class NoopServerLifeCycleListener implements ServerLifeCycleListener { + @Override + public void stopServer(Server server) { + } + + @Override + public void startServer(Server server) { + } + } + + + @Test + public void testServerLifeCycleManager() throws InterruptedException, ExecutionException { + final Random random = new Random(); + final ServerLifeCycleManager lifeCycleManager = new ServerLifeCycleManagerImpl(); + final Server server = new Server() { + + @Override + public void stop() { + } + + @Override + public void start() { + } + + @Override + public boolean isStarted() { + return false; + } + + @Override + public Endpoint getEndpoint() { + return null; + } + + @Override + public Destination getDestination() { + return null; + } + + @Override + public void destroy() { + } + }; + + final NoopServerLifeCycleListener[] listeners = new NoopServerLifeCycleListener[10]; + for (int i = 0; i < 10; ++i) { + listeners[i] = new NoopServerLifeCycleListener(); + lifeCycleManager.registerListener(listeners[i]); + } + + final Phaser phaser = new Phaser(10); + phaser.register(); + + final Collection<Future<?>> futures = new ArrayList<>(); + final ExecutorService executor = Executors.newFixedThreadPool(10); + for (int i = 0; i < 10; ++i) { + final int index = i; + if (random.nextBoolean()) { + futures.add(executor.submit(() -> { + phaser.arriveAndAwaitAdvance(); + lifeCycleManager.stopServer(server); + })); + } else { + futures.add(executor.submit(() -> { + phaser.arriveAndAwaitAdvance(); + lifeCycleManager.unRegisterListener(listeners[index]); + })); + } + } + + phaser.arriveAndDeregister(); + executor.shutdown(); + assertThat(executor.awaitTermination(20, TimeUnit.SECONDS), equalTo(true)); + + for (final Future<?> future: futures) { + assertThat(future.isDone(), equalTo(true)); + assertThat(future.get(), equalTo((Void) null)); + } + } +}
