Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FISH-9171 bugfix: prevent deployment failures from causing an undeployment failure and leaks #6848

Merged
merged 2 commits into from
Aug 21, 2024
Merged

FISH-9171 bugfix: prevent deployment failures from causing an undeployment failure and leaks #6848

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f899f65
bugfix: prevent deployment failures from causing an undeployment fail…
lprimak Aug 3, 2024
1e70b7d
Update appserver/web/web-core/src/main/java/org/apache/catalina/core/…
lprimak Aug 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions appserver/web/web-core/src/main/java/org/apache/catalina/core/StandardContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2303,9 +2303,9 @@ protected void addChild(Container child, boolean isProgrammatic,
Wrapper oldJspServlet = null;

// Allow webapp to override JspServlet inherited from global web.xml.
boolean isJspServlet = "jsp".equals(wrapperName);
boolean isJspServlet = Constants.JSP_SERVLET_NAME.equals(wrapperName);
if (isJspServlet) {
oldJspServlet = (Wrapper) findChild("jsp");
oldJspServlet = (Wrapper) findChild(Constants.JSP_SERVLET_NAME);
if (oldJspServlet != null) {
removeChild(oldJspServlet);
}
Expand Down Expand Up @@ -3238,7 +3238,7 @@ public void addInstanceListener(InstanceListener listener) {
public void addJspMapping(String pattern) {
String servletName = findServletMapping("*.jsp");
if (servletName == null) {
servletName = "jsp";
servletName = Constants.JSP_SERVLET_NAME;
}

if( findChild(servletName) != null) {
Expand Down Expand Up @@ -5766,6 +5766,8 @@ public synchronized void start() throws LifecycleException {
} catch (Throwable t) {
log.log(Level.SEVERE, LogFacade.STARTUP_CONTEXT_FAILED_EXCEPTION, getName());
try {
// ensure that all JSP resources are released in stop() method below
forceLoadJspServlet();
stop();
} catch (Throwable tt) {
log.log(Level.SEVERE, LogFacade.CLEANUP_FAILED_EXCEPTION, tt);
Expand Down Expand Up @@ -5951,7 +5953,11 @@ public synchronized void stop(boolean isShutdown)

if ((manager != null) && (manager instanceof Lifecycle)) {
if(manager instanceof StandardManager) {
((StandardManager)manager).stop(isShutdown);
try {
((StandardManager)manager).stop(isShutdown);
} catch (LifecycleException e) {
log.log(Level.INFO, e.getMessage());
}
} else {
((Lifecycle)manager).stop();
}
Expand Down Expand Up @@ -8132,4 +8138,20 @@ private File getExtractedMetaInfResourcePath(String path) {
}
return null;
}


/**
* Force loading of the JSP servlet. This is needed in case of
* initialization failure, so the JSP servlet would be unloaded properly
* and release all of its resources
*
* @throws ServletException
*/
private void forceLoadJspServlet() throws ServletException {
Container jspServlet = findChild(Constants.JSP_SERVLET_NAME);
if (jspServlet instanceof Wrapper) {
Wrapper jspServletWrapper = (Wrapper) jspServlet;
jspServletWrapper.load();
}
}
}
7 changes: 5 additions & 2 deletions appserver/web/web-glue/src/main/java/com/sun/enterprise/web/WebContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
* only if the new code is made subject to such option by the copyright
* holder.
*/
// Portions Copyright [2016-2022] [Payara Foundation and/or its affiliates]
// Portions Copyright [2016-2024] [Payara Foundation and/or its affiliates]

package com.sun.enterprise.web;

Expand Down Expand Up @@ -232,6 +232,9 @@ public class WebContainer implements org.glassfish.api.container.Container, Post
@Inject
private LoggingRuntime loggingRuntime;

@Inject
private Deployment deployment;

private final Map<String, WebConnector> connectorMap = new HashMap<>();

private EmbeddedWebContainer _embedded;
Expand Down Expand Up @@ -2058,7 +2061,7 @@ public void unloadWebModule(String contextRoot, String appName, String virtualSe
}
}

if (!hasBeenUndeployed) {
if (!hasBeenUndeployed && deployment.getCurrentDeploymentContext() == null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what this meant to be addressing.
&& operator means you're restricting this log message down to an even more specific case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, this message is printed during deployment, and that's an error.
The message is intended to be printed during undeployment only.
This change will check if the deployment process is running, and if it is, it will inhibit this message.

The result is that the message will only be printed during "true" undeployment as it was intended to be originally.

logger.log(Level.SEVERE, LogFacade.UNDEPLOY_ERROR, contextRoot);
}
}
Expand Down