[Fixes #2039] Prevent leaking HttpServiceEnabled instances

(cherry picked from commit 2288d8e)

Author: grgrzybek

pax-web-extender-whiteboard/src/main/java/org/ops4j/pax/web/extender/whiteboard/internal/BundleWhiteboardApplication.java

@@ -157,19 +157,20 @@ public void webContainerRemoved(ServiceReference<WebContainer> ref) {
 
 		// no need to uninstall current contexts and elements - they'll get unregistered when bundle-scoped
 		// HttpService/WebContainer is stopped. Here we only mark them as unregistered
-		WhiteboardWebContainerView view = webContainerManager.whiteboardView(bundle, ref);
-		if (view != null) {
-			webElements.keySet().forEach(element -> {
-				if (element.getContextModels().size() > 0 && webElements.get(element)) {
-					webElements.put(element, false);
-				}
-			});
-			webContexts.keySet().forEach(ctx -> {
-				if (webContexts.get(ctx)) {
-					webContexts.put(ctx, false);
-				}
-			});
-		}
+		webElements.keySet().forEach(element -> {
+			if (element.getContextModels().size() > 0 && webElements.get(element)) {
+				webElements.put(element, false);
+			}
+			if (element instanceof ServletModel && ((ServletModel) element).isResourceServlet()) {
+				// we have to clear up the supplier which is related to old HttpService instance
+				element.setElementSupplier(null);
+			}
+		});
+		webContexts.keySet().forEach(ctx -> {
+			if (webContexts.get(ctx)) {
+				webContexts.put(ctx, false);
+			}
+		});
 
 		webContainerManager.releaseContainer(bundle, ref);
 		webContainerServiceRef = null;

pax-web-jetty/src/main/java/org/ops4j/pax/web/service/jetty/internal/JettyServerWrapper.java

@@ -1286,8 +1286,7 @@ private void removeServletModel(String contextPath, ServletModel model) {
 
 		((PaxWebServletHandler) sch.getServletHandler()).removeServletWithMapping(model);
 
-		LOG.info("Removing servlet {}", model);
-		LOG.debug("Removing servlet {} from context {}", model.getName(), contextPath);
+		LOG.info("Removing servlet {} from context {}", model, contextPath);
 
 		// are there any error page declarations in the model?
 		ErrorPageModel epm = model.getErrorPageModel();

pax-web-jetty/src/main/java/org/ops4j/pax/web/service/jetty/internal/PaxWebServletHandler.java

@@ -205,7 +205,7 @@ protected synchronized void doStop() throws Exception {
 		}
 
 		// Jetty 10+ keeps only "durable" servlets/filters/listeners. We're handling it a bit differently,
-		// so we have to preservet them (because there's no reflection-free access to
+		// so we have to preserve them (because there's no reflection-free access to
 		// org.eclipse.jetty.servlet.ServletHandler._durable field)
 
 		ServletHolder[] servlets = getServlets();

pax-web-runtime/src/main/java/org/ops4j/pax/web/service/internal/HttpServiceEnabled.java

@@ -267,11 +267,13 @@ public void stop() {
 			b.getOperations().add(new TransactionStateChange(OpCode.DISASSOCIATE, ctx));
 		}
 
-		serverModel.runSilently(() -> {
-			serverController.sendBatch(b);
-			b.accept(serviceModel);
-			return null;
-		}, true);
+		if (!b.getOperations().isEmpty()) {
+			serverModel.runSilently(() -> {
+				serverController.sendBatch(b);
+				b.accept(serviceModel);
+				return null;
+			}, true);
+		}
 
 		stopped = true;
 	}
@@ -501,7 +503,9 @@ private void doRegisterServlet(Collection<HttpContext> httpContexts, ServletMode
 						jspSCIModel = serverModel.createJSPServletContainerInitializerModel(serviceBundle);
 						jspSCIModel.setRegisteringBundle(model.getRegisteringBundle());
 						jspSCIModel.getRelatedServletModels().add(model);
-						model.getContextModels().forEach(jspSCIModel::addContextModel);
+						if (!jspSCIModel.hasContextModels()) {
+							model.getContextModels().forEach(jspSCIModel::addContextModel);
+						}
 						newBatch = new Batch("JSP Configuration and registration of " + model);
 
 						// whether or not the target context is started, we're adding an SCI that'll have to be run.
@@ -642,7 +646,7 @@ private void doUnregisterServlet(ServletModel model) {
 										+ "was never registered by " + registeringBundle);
 							}
 						} else if (reference != null) {
-							LOG.info("Unregistering servlet by refernce \"{}\"", reference);
+							LOG.info("Unregistering servlet by reference \"{}\"", reference);
 
 							for (ServletModel existing : serviceModel.getServletModels()) {
 								if (existing.getElementReference().equals(reference)) {
@@ -754,7 +758,7 @@ public void registerResources(String alias, String name, HttpContext context) th
 
 	/**
 	 * Helper method to create a <em>resource servlet</em> using a <em>base</em> which may be either a <em>chroot</em>
-	 * for bundle-resource access of {@code file:} URL.
+	 * for bundle-resource access or {@code file:} URL.
 	 * @param urlPatterns
 	 * @param rawBase
 	 * @return
@@ -983,7 +987,7 @@ private void doUnregisterFilter(final FilterModel model) {
 										+ "was never registered by " + registeringBundle);
 							}
 						} else if (reference != null) {
-							LOG.info("Unregistering filter by refernce \"{}\"", reference);
+							LOG.info("Unregistering filter by reference \"{}\"", reference);
 
 							for (FilterModel existing : serviceModel.getFilterModels()) {
 								if (existing.getElementReference().equals(reference)) {
@@ -1930,7 +1934,9 @@ private void doRegisterWebSocket(List<HttpContext> httpContexts, WebSocketModel
 						cimForGenericWSSupport.setServiceId(0);
 						cimForGenericWSSupport.setServiceRank(0);
 
-						model.getContextModels().forEach(cimForGenericWSSupport::addContextModel);
+						if (!cimForGenericWSSupport.hasContextModels()) {
+							model.getContextModels().forEach(cimForGenericWSSupport::addContextModel);
+						}
 						cimForGenericWSSupport.setRegisteringBundle(model.getRegisteringBundle());
 
 						// now the important part - according to JSR-356 (WebSockets), user is registering endpoints
@@ -1995,7 +2001,9 @@ private ContainerInitializerModel createContainerSpecificWsSCIModel(Bundle bundl
 		cim.setServiceId(0);
 		cim.setServiceRank(Integer.MAX_VALUE);
 
-		model.getContextModels().forEach(cim::addContextModel);
+		if (!cim.hasContextModels()) {
+			model.getContextModels().forEach(cim::addContextModel);
+		}
 		cim.setRegisteringBundle(model.getRegisteringBundle());
 
 		return cim;
@@ -2053,7 +2061,7 @@ private void doUnregisterWebSocket(WebSocketModel model) {
 										+ "was never registered by " + registeringBundle);
 							}
 						} else if (reference != null) {
-							LOG.info("Unregistering webs ocket by refernce \"{}\"", reference);
+							LOG.info("Unregistering webs ocket by reference \"{}\"", reference);
 
 							for (WebSocketModel existing : serviceModel.getWebSocketModels()) {
 								if (existing.getElementReference().equals(reference)) {
@@ -2499,7 +2507,7 @@ public void removeWhiteboardOsgiContextModel(OsgiContextModel model) {
 				try {
 					serverController.sendBatch(batch);
 					batch.accept(serviceModel);
-					handleReRegistrationEvents(WebElementEvent.State.DEPLOYED, batch, null);
+					handleReRegistrationEvents(WebElementEvent.State.UNDEPLOYED, batch, null);
 				} catch (Exception e) {
 					handleReRegistrationEvents(WebElementEvent.State.FAILED, batch, e);
 				}

pax-web-runtime/src/main/java/org/ops4j/pax/web/service/internal/HttpServiceProxy.java

@@ -69,15 +69,20 @@ class HttpServiceProxy implements WebContainer, StoppableHttpService {
 	public void stop() {
 		if (delegate instanceof StoppableHttpService) {
 			LOG.debug("Stopping http service: {}", delegate);
-			final StoppableHttpService stopping = (StoppableHttpService) delegate;
+			StoppableHttpService stopping;
+			synchronized (this) {
+				stopping = (StoppableHttpService) delegate;
 
-			// PAXWEB-1077: ServletContext becomes unavailable on restart when using Whiteboard and CustomContexts
+				// PAXWEB-1077: ServletContext becomes unavailable on restart when using Whiteboard and CustomContexts
 
-			// first replace the delegate
-			delegate = new HttpServiceDisabled(serviceBundle);
+				// first replace the delegate
+				delegate = new HttpServiceDisabled(serviceBundle);
+			}
 
 			// then cleanup the delegate without a risk of problems happening at user side
-			stopping.stop();
+			if (stopping != null) {
+				stopping.stop();
+			}
 		} else {
 			LOG.warn("Http service has already been stopped");
 		}

pax-web-spi/src/main/java/org/ops4j/pax/web/service/spi/servlet/OsgiServletContext.java

@@ -215,9 +215,10 @@ public void register() {
 	public void unregister() {
 		if (registration != null) {
 			try {
-				LOG.info("Unegistering {} as OSGi service for \"{}\" context path", this, osgiContextModel.getContextPath());
+				LOG.info("Unregistering {} as OSGi service for \"{}\" context path", this, osgiContextModel.getContextPath());
 
 				registration.unregister();
+			} catch (IllegalStateException ignored) {
 			} catch (Exception e) {
 				if (osgiContextModel.getOwnerBundle().getState() == Bundle.ACTIVE) {
 					LOG.error("Error unregistering {} from OSGi registry: {}", this, e.getMessage(), e);

pax-web-tomcat/src/main/java/org/ops4j/pax/web/service/tomcat/internal/TomcatServerWrapper.java

@@ -1306,8 +1306,7 @@ public void visitServletModelChange(ServletModelChange change) {
 	}
 
 	private void removeServletModel(String contextPath, ServletModel model) {
-		LOG.info("Removing servlet {}", model);
-		LOG.debug("Removing servlet {} from context {}", model.getName(), contextPath);
+		LOG.info("Removing servlet {} from context {}", model, contextPath);
 
 		// there should already be a ServletContextHandler
 		Context realContext = contextHandlers.get(contextPath);

pax-web-undertow/src/main/java/org/ops4j/pax/web/service/undertow/internal/UndertowServerWrapper.java

@@ -1683,8 +1683,7 @@ public void visitServletModelChange(ServletModelChange change) {
 	private void removeServletModel(String contextPath, ServletModel model, ServletModelChange change) {
 		// this time we just assume that the servlet context is started
 
-		LOG.info("Removing servlet {}", model);
-		LOG.debug("Removing servlet {} from context {}", model.getName(), contextPath);
+		LOG.info("Removing servlet {} from context {}", model, contextPath);
 
 		// take existing deployment manager and the deployment info from its deployment
 		DeploymentManager manager = getDeploymentManager(contextPath);

samples/samples-whiteboard/whiteboard-ds/src/main/java/org/ops4j/pax/web/samples/whiteboard/ds/extended/PaxWebWhiteboardServletMapping.java

@@ -19,23 +19,34 @@
 import java.util.Map;
 import javax.servlet.MultipartConfigElement;
 import javax.servlet.Servlet;
-import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.ops4j.pax.web.service.whiteboard.ServletMapping;
+import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Deactivate;
 
 @Component
 public class PaxWebWhiteboardServletMapping implements ServletMapping {
 
-	private static final Servlet SERVLET = new HttpServlet() {
-		@Override
-		protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
-			resp.getWriter().println("Hello from " + PaxWebWhiteboardServletMapping.class.getName());
-		}
-	};
+	private Servlet servlet = null;
+
+	@Activate
+	protected void activate() {
+		servlet = new HttpServlet() {
+			@Override
+			protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+				resp.getWriter().println("Hello from " + PaxWebWhiteboardServletMapping.class.getName());
+			}
+		};
+	}
+
+	@Deactivate
+	protected void deactivate() {
+		servlet = null;
+	}
 
 	@Override
 	public Class<? extends Servlet> getServletClass() {
@@ -64,7 +75,7 @@ public String getContextId() {
 
 	@Override
 	public Servlet getServlet() {
-		return SERVLET;
+		return servlet;
 	}
 
 	@Override