[jira] [Issue Comment Edited] (WICKET-3737) Remove DynamicImport-Package header from Wicket bundles

["Daniël van 't Ooster (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/WICKET-3737?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13044751#comment-13044751 ] 

Daniël van 't Ooster edited comment on WICKET-3737 at 6/6/11 9:19 AM:
----------------------------------------------------------------------

Hi,

attached is a small set of OSGi bundles, which can be used to reproduce the problem. Also added patches for Wicket and the Wicketstuff wicket-bundle project.

Build instructions:
1. Apply the patches on both wicket and wicket-bundle
2. Build wicket and wicket-bundle
3. Run mvn clean install pax:provision in the OSGi test project dir.

The OSGi test project contains 6 bundles:
com.example.web: contains the Wicket application and has a homepage which displays content provided by bundles com.example.foo and com.example.bar.
com.example.base: contains the interface ComponentProvider, implemented by com.example.foo and com.example.bar to provide components to the homepage.
com.example.foo: provides a very simple panel to the homepage.
com.example.bar: provides a component in which various spring beans are injected.
com.example.classresolver: the only bundle with a DynamicImport-Package. This bundle will be refreshed when a bundle exporting classes used by Wicket is refreshed.
com.example.osgi: OSGi compatible SpringContextLocator and SpringComponentInjector

The homepage shows a list of components provided by com.example.foo and com.example.bar. com.example.bar is exporting the same component twice, so it is possible to see if the request scope works. Some links are added to make it easy to start and stop bundles. The refresh links will refresh parts of the page (does not create a new page instance), and the other link will redirect to a new HomePage instance.

Known issues
* The homepage is broken after refreshing the com.example.bar bundle while clicking through the page. (First: open it, stop and start the bundle and click one of the 'Refresh' links). The page is held in memory, and holds a reference to a bean in a closed applicationContext. It sounds like a memory leak, but no clear idea yet how to solve it.
* I am not sure about the prototype scope, I am never using it in Wicket, and I don't know if people are using it? How is it supposed to work? It seems to hold an memory reference to the same object (which seems to be ok), but this reference will be gone after serialization, so a new bean will be created, which sounds strange when using the prototype scope.

Questions
* In the modified version, an Application.get() call is done in LazyInitProxyFactory.ProxyReplacement.readResolve. Some test cases fail on this, but I did not run into issues while running the application (yet). It is possible to add an Application.exists() check and fall back to the TCCL or bundle classloader, but they will give classloading issues since the DynamicImport-Package header is missing.

grtz,
Daniël

      was (Author: dvantooster):
    Hi,

attached is a small set of OSGi bundles, which can be used to reproduce the problem. Also added patches for Wicket and the Wicketstuff wicket-bundle project.

Build instructions:
1. Apply the patches on both wicket and wicket-bundle
2. Build wicket and wicket-bundle
3. Run mvn clean install pax:provision in the OSGi test project dir.

The OSGi test project contains 5 bundles:
com.example.web: contains the Wicket application and has a homepage which displays content provided by bundles com.example.foo and com.example.bar.
com.example.base: contains the interface ComponentProvider, implemented by com.example.foo and com.example.bar to provide components to the homepage.
com.example.foo: provides a very simple panel to the homepage.
com.example.bar: provides a component in which various spring beans are injected.
com.example.classresolver: the only bundle with a DynamicImport-Package. This bundle will be refreshed when a bundle exporting classes used by Wicket is refreshed.

The homepage shows a list of components provided by com.example.foo and com.example.bar. com.example.bar is exporting the same component twice, so it is possible to see if the request scope works. Some links are added to make it easy to start and stop bundles. The refresh links will refresh parts of the page (does not create a new page instance), and the other link will redirect to a new HomePage instance.

Known issues
* The homepage is broken after refreshing the com.example.bar bundle while clicking through the page. (First: open it, stop and start the bundle and click one of the 'Refresh' links). The page is held in memory, and holds a reference to a bean in a closed applicationContext. It sounds like a memory leak, but no clear idea yet how to solve it.
* I am not sure about the prototype scope, I am never using it in Wicket, and I don't know if people are using it? How is it supposed to work? It seems to hold an memory reference to the same object (which seems to be ok), but this reference will be gone after serialization, so a new bean will be created, which sounds strange when using the prototype scope.

Questions
* In the modified version, an Application.get() call is done in LazyInitProxyFactory.ProxyReplacement.readResolve. Some test cases fail on this, but I did not run into issues while running the application (yet). It is possible to add an Application.exists() check and fall back to the TCCL or bundle classloader, but they will give classloading issues since the DynamicImport-Package header is missing.

grtz,
Daniël
  
> Remove DynamicImport-Package header from Wicket bundles
> -------------------------------------------------------
>
>                 Key: WICKET-3737
>                 URL: https://issues.apache.org/jira/browse/WICKET-3737
>             Project: Wicket
>          Issue Type: Improvement
>          Components: wicket-core
>    Affects Versions: 1.4.17
>            Reporter: Daniël van 't Ooster
>            Assignee: Igor Vaynberg
>              Labels: osgi
>         Attachments: 0001-Removed-DynamicImport-Package-header-from-manifest.patch, WICKET-3737.patch, WICKET-3737_bundles.zip, classloading.patch
>
>
> Hi,
> Wicket can be used in an OSGi container, out of the box it seems to work quite well. However, we experience some problems because of having multiple bundles which depend on Wicket.
> Problem: bundle refreshes caused by DynamicImport-Package
> All Wicket bundles have a DynamicImport-Package: * entry in their manifests. This makes class loading easy, because the class loaders of the Wicket bundles have access to any exported packages from all bundles. This approach has one drawback; the Wicket bundles become implicitly dependent of all bundles which are used at least one time by a Wicket class loader. According to the OSGi specification, when a bundle is refreshed, all bundles which (explicitly or implicitly) depend on that bundle will also be refreshed. 
> Example case: there is an OSGi container with some Wicket bundles loaded, and 2 bundles (bundleA and bundleB), both contain some Wicket components, so they import Wicket packages, they do not import packages from each other. Now we refresh BundleA, we expect only a refresh of only bundleA. In practice all Wicket bundles, bundleA and bundleB are refreshed. This is caused by the DynamicImport-Package, which makes the Wicket bundles implicitly import exported packages from bundleA and bundleB. When bundleA is refreshed, it will refresh also the Wicket bundles. Because of bundleB depends on Wicket, bundleB will also be refreshed. In a small project with a few bundles, this may not be a problem, but it can become a problem when the projects gets larger and you refresh bundles frequently (e.g. when using an OSGi container during development).
> Solution: delegate class loading to another bundle
> While deserializing components, Wicket uses its IClassResolver implementation to load the classes. The DefaultClassResolver uses the thread's context class loader (TCCL) and the classloader of the wicket core bundle (with the DynamicImport-Package). My idea is to remove the DynamicImport-Package header from all Wicket bundles and delegate class loading to another bundle. This bundle exports its OsgiClassResolver (implements IClassResolver) via the OSGi service registry (or a service and reference of Spring DM / Blueprint). This bundle will refresh when bundleA or bundleB is refreshed. Because of there are no bundles which depend on classes from the classResolver bundle, a refresh will not refresh other bundles and will be fast.
> Issues: A few class loading issues
> There are a few spots where we experienced class loading issues. One of the problems is a missing implementation of resolveProxyClass in ObjectInputStream subclasses. Some work is done to solve class loading issues, e.g. by overriding the resolveClass method, but this works for normal classes, proxy classes are handled differently. This is a problem on 2 locations in org.apache.wicket.util.lang.Objects and in org.apache.wicket.util.io.IObjectStreamFactory. Last known issue is in org.apache.wicket.proxy.LazyInitProxyFactory (wicket-ioc), but this one is more complicated. To be able to create a new proxy instance, it needs a classLoader, which can access all interfaces used for the proxy. In a bundle with a DynamicImport, it is safe to pass the classloader of any class in that bundle, but when the DynamicImport-Package is removed, not all classes are visible, and it will throw an exception when one of the classes is not visible. To solve this, I had to extend the IClassResolver interface with one method: getClassLoader(); this classLoader is used when generating the proxy.
> So summarized, the complete solution (based on Spring Dynamic Modules) looks like:
> // applicationContext.xml:
> ...
> <osgi:reference id="classResolver" interface="org.apache.wicket.application.IClassResolver" />
> ...
> <bean id="wicketApplication" class="com.company.WicketApplication"
> 	p:classResolver-ref="classResolver"
> 	...
> />
> ...
> // set up application
> public class WicketApplication extends Application {
>   private IClassResolver classResolver;
>   public void setClassResolver(IClassResolver classResolver) {
>     this.classResolver = classResolver;
>   }
>   @Override
>   public void init() {
>     super.init();
>     IApplicationSettings settings = getApplicationSettings();
>     settings.setClassResolver(this.classResolver);
>   }
> }
> // ClassResolver bundle:
> <bean id="classResolver" class="com.company.osgi.OSGiClassResolver" />
> <osgi:service ref="classResolver">
>   <osgi:interfaces>
>     <value>org.apache.wicket.application.IClassResolver</value>
>   </osgi:interfaces>
> </osgi:service>
> public class OSGiClassResolver extends org.apache.wicket.application.DefaultClassResolver {
>   @Override
>   public ClassLoader getClassLoader() {
>     return OSGiClassResolver.class.getClassLoader();
>   }
> }
> What do you think about this approach? Attached patch is created based on 1.4.10, seems to be compatible with versions up to 1.4.16. 
> best regards,
> Daniël

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira