This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit fc3e17d58d940e2860a6d1e22ead6ef664096126 Author: opencode <[email protected]> AuthorDate: Tue Oct 20 11:07:47 2026 +0200 Bind the initial context of a container in one step The context which the initial context of a container uses is created when it is first needed and is bound to a name built from the name of the object the current thread is bound to. That name was looked up and the new context was bound in two steps, so two threads of a container which needed the context at the same moment could both find that no context was bound yet, both create one and both bind it, the second binding replacing the first. The thread whose context was replaced kept using the context it had created, so the names it bound there were not visible to the other threads of that container, which showed up as a name which was bound successfully and was then not bound. The names concerned are the ones which an application binds through an initial context with a name that does not start with the java: scheme. The bindings the container makes itself are made under the name of the container and are not affected. Read the name and bind the context in a single step instead, with a helper in ContextBindings which binds the context only if no context is bound to that name yet and returns the context which is in use. A thread which finds that another thread was faster then uses the context which was bound rather than the one it created itself. The helper takes the security token as a parameter, like the other methods which bind a context, and it returns the context which is already bound when the caller may not bind anything under that name. --- java/org/apache/naming/ContextBindings.java | 27 ++++ java/org/apache/naming/SelectorContext.java | 10 +- .../naming/TestSelectorContextConcurrency.java | 143 +++++++++++++++++++++ 3 files changed, 177 insertions(+), 3 deletions(-) diff --git a/java/org/apache/naming/ContextBindings.java b/java/org/apache/naming/ContextBindings.java index 066b9c4ec5..231e30e4ad 100644 --- a/java/org/apache/naming/ContextBindings.java +++ b/java/org/apache/naming/ContextBindings.java @@ -105,6 +105,33 @@ public class ContextBindings { } + /** + * Binds an object and a naming context, unless a naming context is already bound to that object, and returns the + * naming context which is in use for that object. The read and the binding are done in a single step, so that a + * caller which finds that another thread bound a context in the meantime uses the context which that thread bound + * rather than the one it created itself. The returned context is never <code>null</code>: it is the context which + * is bound to the object, or the given one when nothing is bound, which happens when the caller may not bind + * anything under that name. + * + * @param obj Object to bind with naming context + * @param context Associated naming context instance + * @param token Security token + * + * @return the naming context which is in use for the object + */ + static Context bindContextIfAbsent(Object obj, Context context, Object token) { + if (!ContextAccessController.checkSecurityToken(obj, token)) { + // Nothing may be bound under that name by this caller, so the + // context which is in use is the one which is already bound, if + // there is one, and the given one otherwise. + Context bound = getContext(obj); + return bound == null ? context : bound; + } + Context bound = objectBindings.putIfAbsent(obj, context); + return bound == null ? context : bound; + } + + /** * Unbinds an object and a naming context. * diff --git a/java/org/apache/naming/SelectorContext.java b/java/org/apache/naming/SelectorContext.java index 8f36da94d7..dc09a3d6e0 100644 --- a/java/org/apache/naming/SelectorContext.java +++ b/java/org/apache/naming/SelectorContext.java @@ -354,9 +354,13 @@ public class SelectorContext implements Context { Context initialContext = ContextBindings.getContext(ICName); if (initialContext == null) { // Allocating a new context and binding it to the appropriate - // name - initialContext = new NamingContext(env, ICName); - ContextBindings.bindContext(ICName, initialContext); + // name. Other threads may do the same for the same name at the + // same time, so the context which is returned is the one which + // ended up being bound rather than the one which was created + // here. Otherwise the bindings made on a context which was not + // bound would not be visible to the other threads using that + // name. + initialContext = ContextBindings.bindContextIfAbsent(ICName, new NamingContext(env, ICName), null); } return initialContext; } else { diff --git a/test/org/apache/naming/TestSelectorContextConcurrency.java b/test/org/apache/naming/TestSelectorContextConcurrency.java new file mode 100644 index 0000000000..e1f4c86c9e --- /dev/null +++ b/test/org/apache/naming/TestSelectorContextConcurrency.java @@ -0,0 +1,143 @@ +/* + * 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.naming; + +import java.util.ArrayList; +import java.util.Enumeration; +import java.util.Hashtable; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicReference; + +import javax.naming.Context; + +import org.junit.Assert; +import org.junit.Test; + +/* + * The context which the initial context of a container uses is created when it + * is first needed and is bound to a name built from the name of the object the + * current thread is bound to. Every thread of the container which needs that + * context at the same moment may find that no context is bound yet and create + * one of its own, and only one of those contexts can be bound. Before the + * context was bound with a single atomic step, the bindings made on the + * contexts which were not bound were lost, which showed up as names which were + * bound successfully and were then not bound. + */ +public class TestSelectorContextConcurrency { + + private static final int THREAD_COUNT = 4; + + private static final String CONTAINER_NAME = "container=/concurrency"; + + + @Test + public void testConcurrentInitialContextCreation() throws Exception { + ContextBindings.bindContext(CONTAINER_NAME, new NamingContext(null, CONTAINER_NAME)); + + try { + CountDownLatch ready = new CountDownLatch(THREAD_COUNT); + CountDownLatch start = new CountDownLatch(1); + AtomicReference<Throwable> bindingFailure = new AtomicReference<>(); + List<Thread> binders = new ArrayList<>(); + + for (int i = 0; i < THREAD_COUNT; i++) { + final String name = "binding" + i; + Thread binder = new Thread(() -> { + try { + ContextBindings.bindThread(CONTAINER_NAME, null); + ready.countDown(); + start.await(); + new SelectorContext(new SlowEnvironment(), true).bind(name, name); + } catch (Throwable t) { + bindingFailure.compareAndSet(null, t); + ready.countDown(); + } finally { + ContextBindings.unbindThread(CONTAINER_NAME, null); + } + }); + binders.add(binder); + binder.start(); + } + + ready.await(); + start.countDown(); + for (Thread binder : binders) { + binder.join(30000); + } + + Assert.assertNull("A thread failed to bind: " + bindingFailure.get(), bindingFailure.get()); + for (Thread binder : binders) { + Assert.assertFalse("A thread did not terminate.", binder.isAlive()); + } + + // Every thread bound a name on the initial context of the container, + // so all of them must be visible from another thread which is bound + // to the same container. The bindings which were made on a context + // which was not bound are not visible. A thread is needed because a + // naming context is bound to the thread which uses it. + AtomicReference<Throwable> lookupFailure = new AtomicReference<>(); + Thread reader = new Thread(() -> { + try { + ContextBindings.bindThread(CONTAINER_NAME, null); + Context initialContext = new SelectorContext(new Hashtable<>(), true); + for (int i = 0; i < THREAD_COUNT; i++) { + String name = "binding" + i; + Assert.assertEquals("The initial context does not have the binding of thread " + i, name, + initialContext.lookup(name)); + } + } catch (Throwable t) { + lookupFailure.compareAndSet(null, t); + } finally { + ContextBindings.unbindThread(CONTAINER_NAME, null); + } + }); + reader.start(); + reader.join(30000); + + Assert.assertFalse("The reader did not terminate.", reader.isAlive()); + Assert.assertNull("The initial context does not have all the bindings: " + lookupFailure.get(), + lookupFailure.get()); + } finally { + ContextBindings.unbindContext(SelectorContext.IC_PREFIX + CONTAINER_NAME, null); + ContextBindings.unbindContext(CONTAINER_NAME, null); + } + } + + + /** + * An environment which is slow to read, so that the threads are still + * copying it into the context they created when the first of them binds + * that context. Without the delay, the moment in which two threads can both + * find that no context is bound is too short for the test to reach it + * reliably. + */ + private static class SlowEnvironment extends Hashtable<String,Object> { + + private static final long serialVersionUID = 1L; + + @Override + public Enumeration<String> keys() { + try { + Thread.sleep(200); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + return super.keys(); + } + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
