Servlet.service called before Servlet.init is finished when servlet is lazily initialized

[sebneter]

Jetty version
9.4.24
Java version
java version "1.8.0_231"
Java(TM) SE Runtime Environment (build 1.8.0_231-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.231-b11, mixed mode)
OS type/version
Windows Server 2016
Description
When a servlet is initialized, there is a small time window during which servlet.init is not yet complete, but servlet.service is already called. I don't know if this is a bug, or if this behavior is desirable. But in earlier versions this never happened. I have tried it with version 9.4.18. There Servlet.service is only called when Servlet.init is completed.

Here is an example code to reproduce the issue. With the first request the servlet is initialized. If this takes some time, then the second request calls Servlet.doGet before Servlet.init is terminated. The output is then:
doGet, uri=/req2
init done
doGet, uri=/req1

import java.io.IOException;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.ws.rs.HttpMethod;

import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.handler.HandlerCollection;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;

public class JettyDemo {

  public static class DemoServlet extends HttpServlet {
    @Override
    public void init() throws ServletException {
      super.init();
      sleep(10000);
      System.out.println("init done");
    }

    @Override
    public void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
      System.out.println("doGet, uri=" + req.getRequestURI());
    }
  }

  private static class Client extends Thread {
    private final String mURI;

    public Client(String uri) {
      mURI = uri;
    }

    @Override
    public void run() {
      try {
        HttpClient client = new HttpClient();
        client.start();
        try {
          client.newRequest("http://127.0.0.1/" + mURI).method(HttpMethod.GET).send();
        } finally {
          client.stop();
        }
      } catch(Exception e) {
        e.printStackTrace();
      }
    }
  }

  private static void sleep(long millis) {
    try {
      Thread.sleep(millis);
    } catch(InterruptedException e) {
      Thread.currentThread().interrupt();
    }
  }

  public static void main(String[] args) throws Exception {
    Server server = new Server();
    HandlerCollection handlers = new HandlerCollection();
    server.setHandler(handlers);
    ServletContextHandler context = new ServletContextHandler(handlers, "/");
    context.addServlet(new ServletHolder(DemoServlet.class), "/*");
    ServerConnector serverConnector = new ServerConnector(server);
    serverConnector.setHost("127.0.0.1");
    serverConnector.setPort(80);
    server.addConnector(serverConnector);
    server.start();
    try {
    new Client("req1").start();
    sleep(5000);
    new Client("req2").start();
    sleep(6000);
    } finally {
      server.stop();
    }
  }

}

Like I said, I don't know if that's a bug. But as a user I would find it very helpful if I could assume that all my resources initialized in Servlet.init are safely ready when Servlet.service is called and I don't have to synchronize them myself.

[joakime]

This discovery might explain what we saw (once) in #4327

[janbartel]

Looks like we changed the code in commit d48c258#diff-0d419a5cace2d3e164cd481d96f24734 that changed the way we do servlet initialization. When 2 requests come in simultaneously, they are only both checking if the servlet instance is non-null: one will create the instance and then go on to initialize it, meanwhile the second request sees the created-but-not-yet-completely-initialized instance and uses it. Working on a fix.

[gregw]

So this is a real problem... in which case we should say that the work around is to mark your servlets as initialise on startup.

[janbartel]

Raised PR #4354.

[janbartel]

Fix committed in time for 9.4.25.