Merge pull request #4354 from eclipse/jetty-9.4.x-4351-lazy-servlet-i…

…nit-not-atomic

Issue #4351 Lazy servlet init is not atomic

jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java

@@ -470,21 +470,16 @@ public void destroyInstance(Object o)
     public Servlet getServlet()
         throws ServletException
     {
-        Servlet servlet = _servlet;
-        if (servlet == null)
+        synchronized (this)
         {
-            synchronized (this)
+            if (_servlet == null && isRunning())
             {
-                servlet = _servlet;
-                if (servlet == null && isRunning())
-                {
-                    if (getHeldClass() != null)
-                        initServlet();
-                    servlet = _servlet;
-                }
+                if (getHeldClass() != null)
+                    initServlet();
             }
         }
-        return servlet;
+
+        return _servlet;
     }
 
     /**
@@ -626,7 +621,7 @@ protected void initJspServlet() throws Exception
         ContextHandler ch = ContextHandler.getContextHandler(getServletHandler().getServletContext());
 
         /* Set the webapp's classpath for Jasper */
-        ch.setAttribute("org.apache.catalina.jspgetHeldClass()path", ch.getClassPath());
+        ch.setAttribute("org.apache.catalina.jsp_classpath", ch.getClassPath());
 
         /* Set up other classpath attribute */
         if ("?".equals(getInitParameter("classpath")))

jetty-servlet/src/test/java/org/eclipse/jetty/servlet/InitServletTest.java

@@ -0,0 +1,132 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.servlet;
+
+import java.io.IOException;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.api.Response;
+import org.eclipse.jetty.client.api.Result;
+import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.server.NetworkConnector;
+import org.eclipse.jetty.server.Server;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class InitServletTest
+{
+    public static class DemoServlet extends HttpServlet
+    {
+        private static final long INIT_SLEEP = 2000;
+        private final AtomicInteger initCount = new AtomicInteger();
+
+        @Override
+        public void init() throws ServletException
+        {
+            super.init();
+            try
+            {
+                // Make the initialization last a little while.
+                // Other requests must wait.
+                Thread.sleep(INIT_SLEEP);
+            }
+            catch (InterruptedException e)
+            {
+                throw new ServletException(e);
+            }
+            initCount.incrementAndGet();
+        }
+
+        @Override
+        public void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
+        {
+            // Check that the init() method has been totally finished (by another request)
+            // before the servlet service() method is called.
+            if (initCount.get() != 1)
+                resp.sendError(500, "Servlet not initialized!");
+        }
+    }
+
+    private static class AsyncResponseListener implements Response.CompleteListener
+    {
+        private final AtomicInteger index = new AtomicInteger();
+        private final CountDownLatch resultsLatch;
+        private final int[] results;
+
+        public AsyncResponseListener(CountDownLatch resultsLatch, int[] results)
+        {
+            this.resultsLatch = resultsLatch;
+            this.results = results;
+        }
+
+        public void onComplete(Result result)
+        {
+            results[index.getAndIncrement()] = result.getResponse().getStatus();
+            resultsLatch.countDown();
+        }
+    }
+
+    @Test
+    public void testServletInitialization() throws Exception
+    {
+        Server server = new Server(0);
+        ServletContextHandler context = new ServletContextHandler(server, "/");
+        server.setHandler(context);
+        // Add a lazily instantiated servlet.
+        context.addServlet(new ServletHolder(DemoServlet.class), "/*");
+        HttpClient client = new HttpClient();
+        server.addBean(client);
+        server.start();
+        try
+        {
+            int port = ((NetworkConnector)server.getConnectors()[0]).getLocalPort();
+
+            // Expect 2 responses
+            CountDownLatch resultsLatch = new CountDownLatch(2);
+            int[] results = new int[2];
+            AsyncResponseListener l = new AsyncResponseListener(resultsLatch, results);
+
+            // Req1: should initialize servlet.
+            client.newRequest("http://localhost:" + port + "/r1").send(l);
+
+            // Need to give 1st request a head start before request2.
+            Thread.sleep(DemoServlet.INIT_SLEEP / 4);
+
+            // Req2: should see servlet fully initialized by request1.
+            client.newRequest("http://localhost:" + port + "/r2").send(l);
+
+            assertTrue(resultsLatch.await(DemoServlet.INIT_SLEEP * 2, TimeUnit.MILLISECONDS));
+            assertEquals(HttpStatus.OK_200, results[0]);
+            assertEquals(HttpStatus.OK_200, results[1]);
+        }
+        finally
+        {
+            server.stop();
+        }
+    }
+}