search

ops4j / org.ops4j.pax.web

OSGi R7 Http Service, Whiteboard and Web Applications (OSGi CMPN Release chapters 102, 140 and 128) implementation using Jetty 9, Tomcat 9 or Undertow 2.
https://ops4j1.jira.com/wiki/display/paxweb/Pax+Web
Other
144 stars 183 forks source link

[whiteboard] incomplete unregistering of web elements in shared context #1603

Closed mfriedemann closed 3 years ago

mfriedemann commented 3 years ago

Hey,

we identified an issue in the whiteboard extender of pax-web with unregistering elements from a shared context.

In the case of a shared context, AbstractTracker will only remove web elements from the application when it deems the whole application about to be removed. https://github.com/ops4j/org.ops4j.pax.web/blob/7cbd5dbcc1d4b6c0642d6edc8a7b10dad9584e89/pax-web-extender-whiteboard/src/main/java/org/ops4j/pax/web/extender/whiteboard/internal/tracker/AbstractTracker.java#L244-L245

The shared context counting done there appears itself a bit strange (it has a special case the other trackers do not use), but ignoring that, the elements should be removed from the app in any case. Otherwise they will stay registered with the web container and essentially cause issues later, eg. when trying to re-register the same elements (from an updated application archive, for instance).

Additionally, due to the differences in the trackers handling the shared context, the application might never be unregistered from the context, leaving that dangling and also causing issues with re-registering later. https://github.com/ops4j/org.ops4j.pax.web/blob/7cbd5dbcc1d4b6c0642d6edc8a7b10dad9584e89/pax-web-extender-whiteboard/src/main/java/org/ops4j/pax/web/extender/whiteboard/internal/tracker/ServletContextHelperTracker.java#L219-L220

A related issue (#1368) was discussed before, but I am not about the actual outcome.

I have built a reduced example of whiteboard components using OSGi declarative services that shows the issue when deployed into a karaf with pax-web (with 7.2.14 and also 7.3.13 from latest karaf-4.3.1).

The following patch (built and tested against 7.2.14, but should apply to later versions as well) resolves both issues, by making sure the application resets the context mapping, and by removing all web elements from the application. https://gist.github.com/mfriedemann/1ed1ac18e50c9fff8751fd1a966bf3c5

Regards, M.

grgrzybek commented 3 years ago

Hello

Great analysis Marko!

To be honest, I'm still refactoring Pax Web 8, which completely changes the way how Whiteboard/HttpService/WarExtender aspects are working.

In Pax Web 8 org.ops4j.pax.web.extender.whiteboard.internal.WebApplication class:

private final Map<OsgiContextModel, Boolean> webContexts = new LinkedHashMap<>();

It's all about the model, which in Pax web 7 was sufficient, but wrong.

In Pax Web 8, a "web application", or rather "a collection of whiteboard services registered by single bundle" isn't related 1:1 with ServletContextHelper, but rather 1:N...

This changes everything. Also any element, like servlet, filter or listener may be (while still being registered by single bundle) targeted to multiple ServletContextHelpers, and effectively to different (javax.servlet.)ServletContexts... This wasn't possible in Pax Web 7.

I have lots of tests in Pax Web 8 showing what happens if you for example change the context path property of already registered ServletContextHelper

Important aspect of Pax Web 8 is cleanup - when HttpService instance is gone or the bundle registering any whiteboard service is gone, everything is cleaned up.

If your patch works for you and you've run all the integration tests for Pax Web 7.2/7.3/7.4 then I think we could merge it, but expect complete rewrite with Pax Web 8, which I hope to release at some point...

regards Grzegorz Grzybek

pon., 17 maj 2021 o 13:28 Marko Friedemann @.***> napisał(a):

Hey,

we identified an issue in the whiteboard extender of pax-web with unregistering elements from a shared context.

In the case of a shared context, AbstractTracker will only remove web elements from the application when it deems the whole application about to be removed.

https://github.com/ops4j/org.ops4j.pax.web/blob/7cbd5dbcc1d4b6c0642d6edc8a7b10dad9584e89/pax-web-extender-whiteboard/src/main/java/org/ops4j/pax/web/extender/whiteboard/internal/tracker/AbstractTracker.java#L244-L245

The shared context counting done there appears itself a bit strange (it has a special case the other trackers do not use), but ignoring that, the elements should be removed from the app in any case. Otherwise they will stay registered with the web container and essentially cause issues later, eg. when trying to re-register the same elements (from an updated application archive, for instance).

Additionally, due to the differences in the trackers handling the shared context, the application might never be unregistered from the context, leaving that dangling and also causing issues with re-registering later.

https://github.com/ops4j/org.ops4j.pax.web/blob/7cbd5dbcc1d4b6c0642d6edc8a7b10dad9584e89/pax-web-extender-whiteboard/src/main/java/org/ops4j/pax/web/extender/whiteboard/internal/tracker/ServletContextHelperTracker.java#L219-L220

A related issue (#1368 https://github.com/ops4j/org.ops4j.pax.web/issues/1368) was discussed before, but I am not about the actual outcome.

I have built a reduced example https://github.com/mfriedemann/osgi-ds-whiteboard-components of whiteboard components using OSGi declarative services that shows the issue when deployed into a karaf with pax-web (with 7.2.14 and also 7.3.13 from latest karaf-4.3.1).

The following patch (built and tested against 7.2.14, but should apply to later versions as well) resolves both issues, by making sure the application resets the context mapping, and by removing all web elements from the application. https://gist.github.com/mfriedemann/1ed1ac18e50c9fff8751fd1a966bf3c5

Regards, M.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ops4j/org.ops4j.pax.web/issues/1603, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACK3BIDTM2PWM3HB7YV3LLTOD4W3ANCNFSM45AIQAEQ .

mfriedemann commented 3 years ago

Hello Grzegorz,

thank you for the quick response.

I don't really know much about the inner workings of pax-web, I just dug into it enough to identify this issue. So as far as your rework for v8 goes, I cannot say what implications my findings might have.

In a nutshell, we are mere users of the whiteboard services to register a number of components that make up our web frontend. The specific combination of elements (filter, servlet helper, resources) is apparently something that triggers this issue, and while the tests that are there in pax-web don't fail (they do emit some warnings, though), the real deployment does not work (well, restarting/redeploying doesn't work)

If there is one thing that comes off this effort to fix the issue, may I please ask you to check the example bundle I prepared, and the combination of elements it registers, against your rework to ensure that it can be deployed, stopped, restarted, redeployed etc. in a v8 whiteboard without issues? Maybe you can integrate a similar combination of elements as a new testcase directly? I just stripped down and simplified what we have, to be able to present some minimal case that exhibits the issue.

Thanks, M.

To some background: We switched an application to karaf last year to be able to deploy updated versions but found that due to a number of issues with our own code, plus the reported issue in the whiteboard, we can't actually redeploy dynamically and need to restart the whole thing. While unfortunate, we had been able to cope with that for a while, but need to work on a solution now because other people (on-site) tend to one of those installations now and we need to make the whole update/maintenance process more easy and less error-prone.

grgrzybek commented 3 years ago

I'll remember to check your example this week, ok?

regards Grzegorz Grzybek

pon., 17 maj 2021 o 15:19 Marko Friedemann @.***> napisał(a):

Hello Grzegorz,

thank you for the quick response.

I don't really know much about the inner workings of pax-web, I just dug into it enough to identify this issue. So as far as your rework for v8 goes, I cannot say what implications my findings might have.

In a nutshell, we are mere users of the whiteboard services to register a number of components that make up our web frontend. The specific combination of elements (filter, servlet helper, resources) is apparently something that triggers this issue, and while the tests that are there in pax-web don't fail (they do emit some warnings, though), the real deployment does not work (well, restarting/redeploying doesn't work)

If there is one thing that comes off this effort to fix the issue, may I please ask you to check the example bundle I prepared, and the combination of elements it registers, against your rework to ensure that it can be deployed, stopped, restarted, redeployed etc. in a v8 whiteboard without issues? Maybe you can integrate a similar combination of elements as a new testcase directly? I just stripped down and simplified what we have, to be able to present some minimal case that exhibits the issue.

Thanks, M.

To some background: We switched an application to karaf last year to be able to deploy updated versions but found that due to a number of issues with our own code, plus the reported issue in the whiteboard, we can't actually redeploy dynamically and need to restart the whole thing. While unfortunate, we had been able to cope with that for a while, but need to work on a solution now because other people (on-site) tend to one of those installations now and we need to make the whole update/maintenance process more easy and less error-prone.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ops4j/org.ops4j.pax.web/issues/1603#issuecomment-842318142, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACK3BPQZAPJDXW72TU5EATTOEJXZANCNFSM45AIQAEQ .

mfriedemann commented 3 years ago

Sure, thank you.

Best regards, M.

grgrzybek commented 3 years ago

Sorry for so long delay...

I've just backported your example to Pax Web 8 tests and ... everything is fine ;) Which means I did correct resource management with complex(ish) Whiteboard registrations. In fact, I've prepared even more complex integration test, where mutliple elements (servlets, filters, resources) are registered into multiple servlet contexts (then select up to 4 ServletContextHelper services).

With Pax Web 8, after the bundle with SCR components is stopped, I see this state of memory:

HttpServiceEnabled@5654 = "HttpService (enabled) for bundle org.ops4j.pax.web.samples.whiteboard-ds-1603 [42]"
 configuration: org.ops4j.pax.web.service.spi.config.Configuration  = {org.ops4j.pax.web.service.internal.ConfigurationImpl@5666} 
 directContainerView: org.ops4j.pax.web.service.internal.views.DirectWebContainerView  = {org.ops4j.pax.web.service.internal.HttpServiceEnabled$DirectWebContainer@5681} 
 eventDispatcher: org.ops4j.pax.web.service.spi.model.events.WebElementEventListener  = {org.ops4j.pax.web.service.internal.WebElementEventDispatcher@5662} 
 serverController: org.ops4j.pax.web.service.spi.ServerController  = {org.ops4j.pax.web.service.jetty.internal.JettyServerController@5661} "JettyServerController{configuration=71944f23-53af-45d4-ab70-423fe5d19ed2,state=STARTED}"
 serverModel: org.ops4j.pax.web.service.spi.model.ServerModel  = {org.ops4j.pax.web.service.spi.model.ServerModel@5659} 
 serviceBundle: org.osgi.framework.Bundle  = {org.apache.felix.framework.BundleImpl@5652} "org.ops4j.pax.web.samples.whiteboard-ds-1603 [42]"
 serviceModel: org.ops4j.pax.web.service.spi.model.ServiceModel  = {org.ops4j.pax.web.service.spi.model.ServiceModel@5679} 
  aliasMapping: java.util.Map  = {java.util.HashMap@5685}  size = 0
  containerInitializerModels: java.util.Set  = {java.util.HashSet@5689}  size = 0
  errorPageModels: java.util.Set  = {java.util.HashSet@5692}  size = 0
  eventListenerModels: java.util.Set  = {java.util.HashSet@5688}  size = 0
  filterModels: java.util.Set  = {java.util.HashSet@5687}  size = 0
  serverController: org.ops4j.pax.web.service.spi.ServerController  = {org.ops4j.pax.web.service.jetty.internal.JettyServerController@5661} "JettyServerController{configuration=71944f23-53af-45d4-ab70-423fe5d19ed2,state=STARTED}"
  serverModel: org.ops4j.pax.web.service.spi.model.ServerModel  = {org.ops4j.pax.web.service.spi.model.ServerModel@5659} 
  serviceBundle: org.osgi.framework.Bundle  = {org.apache.felix.framework.BundleImpl@5652} "org.ops4j.pax.web.samples.whiteboard-ds-1603 [42]"
  servletModels: java.util.Set  = {java.util.HashSet@5686}  size = 0
  welcomeFileModels: java.util.Set  = {java.util.HashSet@5691}  size = 0
  welcomeFiles: java.util.Map  = {java.util.LinkedHashMap@5690}  size = 0

The tests that uses your example is in https://github.com/ops4j/org.ops4j.pax.web/blob/4fa2f687d54e1017f86706633a27ffb9f9c4c55d/pax-web-itest/pax-web-itest-container/pax-web-itest-container-common/src/main/java/org/ops4j/pax/web/itest/container/whiteboard/AbstractWhiteboardDS1603IntegrationTest.java The example (thanks @mfriedemann for it!) is in https://github.com/ops4j/org.ops4j.pax.web/tree/4fa2f687d54e1017f86706633a27ffb9f9c4c55d/samples/samples-whiteboard/whiteboard-ds-1603

I've checked your patch at https://gist.github.com/mfriedemann/1ed1ac18e50c9fff8751fd1a966bf3c5 and ... there's nothing we can do at this stage in Pax Web 7... In fact, org.ops4j.pax.web.extender.whiteboard.internal.WebApplication#setHttpContextMapping() method and two issues:

where the reasons I started to rewrite Pax Web more than 2 years ago (in my kind of spare time)...

The problem in Pax Web 7 is with the fundamental model of the entities. Every bundle may install (register) many ServletContextHelper services and all should be tracked and all should be cleaned up. WebApplication#setHttpContextMapping() call assumes there's always one ServletContextHelper per "web application" which is fundamentally wrong...

For now I really want to finish and release Pax Web 8...

Recently I was able to run Vaadin WARs (with and without Spring Boot) and even MyFaces + AriesCDI + JSP + WAB examples work! So the release is on the horizon.