create action optimization

core/src/main/java/org/apache/struts/action/RequestProcessor.java

@@ -22,7 +22,8 @@
 
 import java.io.IOException;
 import java.io.Serializable;
-import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.Locale;
 
 import jakarta.servlet.RequestDispatcher;
@@ -86,7 +87,7 @@ public class RequestProcessor implements Serializable {
      * initialized, keyed by the fully qualified Java class name of the
      * <code>Action</code> class.</p>
      */
-    protected HashMap<String, Action> actions = new HashMap<>();
+    protected final Map<String, Action> actions = new ConcurrentHashMap<>();
 
     /**
      * <p>The <code>ModuleConfiguration</code> with which we are
@@ -125,9 +126,7 @@ public void destroy() {
      */
     public void init(ActionServlet servlet, ModuleConfig moduleConfig)
         throws ServletException {
-        synchronized (actions) {
-            actions.clear();
-        }
+        actions.clear();
 
         this.servlet = servlet;
         this.moduleConfig = moduleConfig;
@@ -198,9 +197,7 @@ public void process(HttpServletRequest request, HttpServletResponse response)
             ActionForward forward = processException(request, response, e, form, mapping);
             processForwardConfig(request, response, forward);
             return;
-        } catch (IOException e) {
-            throw e;
-        } catch (ServletException e) {
+        } catch (IOException | ServletException e) {
             throw e;
         }
 
@@ -241,9 +238,11 @@ public void process(HttpServletRequest request, HttpServletResponse response)
      *         the current request.
      * @throws IOException if an input/output error occurs
      */
-    protected Action processActionCreate(HttpServletRequest request,
-        HttpServletResponse response, ActionMapping mapping)
-        throws IOException {
+    protected Action processActionCreate(
+            HttpServletRequest request,
+            HttpServletResponse response,
+            ActionMapping mapping
+    ) throws IOException {
         // Acquire the Action instance we will be using (if there is one)
         String className = mapping.getType();
 
@@ -252,21 +251,11 @@ protected Action processActionCreate(HttpServletRequest request,
         // If there were a mapping property indicating whether
         // an Action were a singleton or not ([true]),
         // could we just instantiate and return a new instance here?
-        Action instance;
-
-        synchronized (actions) {
-            // Return any existing Action instance of this class
-            instance = actions.get(className);
-
-            if (instance != null) {
-                log.trace("  Returning existing Action instance");
-
-                return (instance);
-            }
-
+        Action action = actions.computeIfAbsent(className, key -> {
             // Create and return a new Action instance
             log.trace("  Creating new Action instance");
 
+            Action instance;
             try {
                 instance = (Action) RequestUtils.applicationInstance(className);
 
@@ -278,20 +267,24 @@ protected Action processActionCreate(HttpServletRequest request,
                         mapping.getPath(), mapping.toString()))
                     .setCause(e).log();
 
-                response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
-                    getInternal().getMessage("actionCreate", mapping.getPath()));
-
                 return (null);
             }
 
-            actions.put(className, instance);
-
             if (instance.getServlet() == null) {
                 instance.setServlet(this.servlet);
             }
+
+            return instance;
+        });
+
+        if (action == null) {
+            response.sendError(
+                    HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+                    getInternal().getMessage("actionCreate", mapping.getPath())
+            );
         }
 
-        return (instance);
+        return (action);
     }
 
     /**

core/src/main/java/org/apache/struts/chain/commands/servlet/CreateAction.java

@@ -20,9 +20,6 @@
  */
 package org.apache.struts.chain.commands.servlet;
 
-import java.util.HashMap;
-import java.util.Map;
-
 import org.apache.struts.action.Action;
 import org.apache.struts.action.ActionServlet;
 import org.apache.struts.chain.Constants;
@@ -31,9 +28,13 @@
 import org.apache.struts.chain.contexts.ServletActionContext;
 import org.apache.struts.config.ActionConfig;
 import org.apache.struts.config.ModuleConfig;
+import org.apache.struts.util.Try;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
 /**
  * <p>Concrete implementation of <code>AbstractCreateAction</code> for use in
  * a Servlet API chain.  Expects that the ActionContext passed into it can
@@ -52,34 +53,38 @@ public class CreateAction
     /* :TODO The Action class' dependency on having its "servlet" property set
      * requires this API-dependent subclass of AbstractCreateAction.
      */
-    protected synchronized Action getAction(ActionContext context, String type,
-        ActionConfig actionConfig)
-        throws Exception {
+    protected Action getAction(
+            ActionContext context,
+            String type,
+            ActionConfig actionConfig
+    ) throws Exception {
 
         ServletActionContext saContext = (ServletActionContext) context;
         ActionServlet actionServlet = saContext.getActionServlet();
 
         ModuleConfig moduleConfig = actionConfig.getModuleConfig();
         String actionsKey = Constants.ACTIONS_KEY + moduleConfig.getPrefix();
         @SuppressWarnings("unchecked")
-        Map<String, Action> actions = (Map<String, Action>) context.getApplicationScope().get(actionsKey);
+        Map<String, Try<Action>> actions = (Map<String, Try<Action>>) context.getApplicationScope().get(actionsKey);
 
         if (actions == null) {
-            actions = new HashMap<>();
-            context.getApplicationScope().put(actionsKey, actions);
+            synchronized (this) {
+                //noinspection unchecked
+                actions = (Map<String, Try<Action>>) context.getApplicationScope().get(actionsKey);
+                if (actions == null) {
+                    actions = new ConcurrentHashMap<>();
+                    context.getApplicationScope().put(actionsKey, actions);
+                }
+            }
         }
 
-        Action action = null;
+        Action action;
 
         try {
             if (actionConfig.isSingleton()) {
-                synchronized (actions) {
-                    action = actions.get(type);
-                    if (action == null) {
-                        action = createAction(context, type);
-                        actions.put(type, action);
-                    }
-                }
+                action = actions
+                        .computeIfAbsent(type, key -> Try.ofCallable(() -> createAction(context, type)))
+                        .get();
             } else {
                 action = createAction(context, type);
             }

core/src/main/java/org/apache/struts/chain/contexts/ActionContext.java

@@ -38,9 +38,9 @@
  * session-scoped resources and services</p>
  */
 public interface ActionContext extends Context {
-    public static final String APPLICATION_SCOPE = "application";
-    public static final String SESSION_SCOPE = "session";
-    public static final String REQUEST_SCOPE = "request";
+    String APPLICATION_SCOPE = "application";
+    String SESSION_SCOPE = "session";
+    String REQUEST_SCOPE = "request";
 
     // -------------------------------
     // General Application Support

core/src/main/java/org/apache/struts/util/Try.java

@@ -0,0 +1,94 @@
+/*
+ * $Id$
+ *
+ * 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.struts.util;
+
+import java.util.concurrent.Callable;
+
+/**
+ * The {@code Try} class represents a computation that may result in a successfully computed value
+ * or a checked exception.
+ *
+ * @param <T> the type of the result produced by the computation
+ */
+public abstract class Try<T> {
+
+    /**
+     * Wraps a checked computation into a {@code Try} instance. If the computation throws an exception,
+     * a {@code Failure} containing the exception is returned; otherwise, a {@code Success} with the
+     * result is returned.
+     *
+     * @param callable the checked computation to execute
+     * @param <T>      the result type of the computation
+     * @return a {@code Try} representing the success or failure of the computation
+     */
+    public static <T> Try<T> ofCallable(Callable<T> callable) {
+        try {
+            return new Success<>(callable.call());
+        } catch (Exception e) {
+            return new Failure<>(e);
+        }
+    }
+
+    /**
+     * Returns the result value if the computation is successful. If this is a {@code Failure}, throws
+     * the contained exception.
+     *
+     * @return the successfully computed value
+     * @throws Exception if this is a {@code Failure}, wrapping the original checked exception
+     */
+    public abstract T get() throws Exception;
+
+    /**
+     * Represents result of a successfully computed value
+     *
+     * @param <T> the type of the result produced by the computation
+     */
+    public static class Success<T> extends Try<T> {
+        private final T result;
+
+        public Success(T result) {
+            this.result = result;
+        }
+
+        @Override
+        public T get() {
+            return result;
+        }
+    }
+
+    /**
+     * Represents an exception
+     *
+     * @param <T> the type of the result produced by the computation
+     */
+    public static class Failure<T> extends Try<T> {
+        private final Exception e;
+
+        public Failure(Exception e) {
+            this.e = e;
+        }
+
+        @Override
+        public T get() throws Exception {
+           throw e;
+        }
+    }
+}