Skip to content

Commit

Permalink
Try to eager load Servlets at startup.
Browse files Browse the repository at this point in the history
This will trigger all the required CDI events, with as side-effect that
we also created the Servlet in advance.

According to the Servlet spec, this is allowed, and the Servlet TCK
indeed passes with this change.

Signed-off-by: Arjan Tijms <arjan.tijms@gmail.com>
  • Loading branch information
arjantijms committed Jun 6, 2022
1 parent 07a46a9 commit 50ed13a
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,8 @@ public interface Wrapper extends Container {
*/
public void load() throws ServletException;

public void tryLoad() throws ServletException;


/**
* Remove the specified initialization parameter from this servlet.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4977,37 +4977,58 @@ public boolean alternateResourcesStop() {
*/
public void loadOnStartup(Container children[]) throws LifecycleException {
// Collect "load on startup" servlets that need to be initialized
Map<Integer, List<Wrapper>> map = new TreeMap<>();
Map<Integer, List<Wrapper>> loadOnStartupServlets = new TreeMap<>();
List<Wrapper> nonLoadOnStartupServlets = new ArrayList<>();

for (Container aChildren : children) {
Wrapper wrapper = (Wrapper) aChildren;

int loadOnStartup = wrapper.getLoadOnStartup();
if (loadOnStartup < 0) {
continue;
nonLoadOnStartupServlets.add(wrapper);
} else {
loadOnStartupServlets.computeIfAbsent(loadOnStartup, e -> new ArrayList<>())
.add(wrapper);
}
}

Integer key = loadOnStartup;
List<Wrapper> list = map.get(key);
if (list == null) {
list = new ArrayList<>();
map.put(key, list);
// Combine the load on startup and non load on startup in one list, with the
// latter loading after the ones with an explicit priority (load level).
List<Wrapper> allServlets = new ArrayList<>();
for (List<Wrapper> samePriorityServlets : loadOnStartupServlets.values()) {
for (Wrapper wrapper : samePriorityServlets) {
allServlets.add(wrapper);
}
list.add(wrapper);
}

// Load the collected "load on startup" servlets
for (List<Wrapper> list : map.values()) {
for (Wrapper wrapper : list) {
try {
wrapper.load();
} catch (ServletException e) {
getServletContext().log(
format(rb.getString(SERVLET_LOAD_EXCEPTION), neutralizeForLog(getName())),
StandardWrapper.getRootCause(e));
for (Wrapper wrapper : allServlets) {
try {
wrapper.load();
} catch (ServletException e) {
getServletContext().log(
format(rb.getString(SERVLET_LOAD_EXCEPTION), neutralizeForLog(getName())),
StandardWrapper.getRootCause(e));

// NOTE: load errors (including a servlet that throws
// UnavailableException from the init() method) are NOT
// fatal to application startup
throw new LifecycleException(StandardWrapper.getRootCause(e));
}
}

if (Boolean.parseBoolean(System.getProperty("glassfish.try.preload.servlets", "true"))) {
// Also load the other servlets, which is one way to pass the CDI TCK, specifically
// ContainerEventTest#testProcessInjectionTargetEventFiredForServlet and adhere to the rule
// that injection points for Servlets have to be processed during start.
for (Wrapper wrapper : nonLoadOnStartupServlets) {
try {
wrapper.tryLoad();
} catch (Throwable t) {
// Only log errors, don't fail anything.
getServletContext().log(
format(rb.getString(SERVLET_LOAD_EXCEPTION), neutralizeForLog(getName())),
t);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,23 @@ public synchronized void load() throws ServletException {
initServlet(instance);
}

@Override
public synchronized void tryLoad() throws ServletException {
try {
instance = loadServlet();
initServlet(instance);
} catch (Throwable t) {
instance = null;
available = 0l;
classLoadTime = 0;
loadTime = 0l;
instanceInitialized = false;

throw t;
}
}


/**
* Creates an instance of the servlet, if there is not already at least one initialized instance.
*/
Expand Down

0 comments on commit 50ed13a

Please sign in to comment.