Hi, I like the principle behind this change, and think it probably is a good way to go. It's lightweight, and results can be cached. There is one negative point that affects this (and in fact almost all) filtering, wihch is that the the weaving code is built around the fact that we weave consistently throughout the class hierarchy up to a certain ancestor (preferably java.lang.Object). I find this quite difficult to explain well in words, so forgive the pretty trivial example.
We have three classes: Foo extends Bar, Bar extends Jimmy, and Jimmy extends Object. When we are weaving Foo, the first thing we do is to load Bar. If Bar can be woven then we will start by loading Jimmy. If Jimmy can be woven then we start by loading Object (which cannot be woven). If all three classes can be woven then we add hooks into every class, which is fine. If we can't weave Jimmy (which sometimes happens), then when we weave Bar we can detect that Jimmy wasn't woven (because it doesn't implement WovenProxy), and add overrides in Bar for any methods that it inherits from Jimmy to allow us to weave in code. This means that Jimmy can't be proxied (by the weaving code) but that Foo and Bar can. The problem is that this detection logic only spots the first break in the weaving chain, and not subsequent breaks. If we were able to weave Foo and Jimmy, but not Bar, then the weaving code would not know to override the methods that Foo inherits from Bar (Bar implements WovenProxy because Jimmy does). Even worse, the weaving code adds some synthetic methods to every class in the weaving hierarchy. These will be missing from Bar, and use the implementations from Jimmy instead, which will give the wrong answer. Also, because Bar will still implements the WovenProxy interface the proxy bundle will think that Bar is proxyable, even though it actually isn't. I don't necessarily see this issue as a dealbreaker for the proposed solution, but I do want people to be aware of the risks associated with only weaving a subset of classes in a hierarchy. We could potentially prevent this situation by ignoring the bundle-wide filter if the superclass of the class to be woven already implements WovenProxy. This would mean that some classes from "unweavable" bundles would be woven, but only if they were already part of a woven hierarchy from some other bundle. It seems highly unlikely that this would affect Alasdair's scenario (which I imagine is the common use case), but it also prevents us from ever violating the assumptions made by the weaving code about the continuity of the woven methods. All in all, a tentative +1. Alasdair, what do you think of a minor tweak to override the filter in the case I describe above? I think it will be a line or two change to your existing code, plus a very simple ClassVisitor. We could even move some of the code out of the visit method of AbstractWovenProxyAdapter and pass the super class as a java.lang.Class constructor parameter to avoid duplicating effort. With this change I would have no concerns about the solution at all. Regards Tim Ward ------------------- Apache Aries PMC member & Enterprise OSGi advocate Enterprise OSGi in Action (http://www.manning.com/cummins) ------------------- > Date: Sat, 4 Feb 2012 23:43:38 +0000 > Subject: Customising the behaviour of the weaving proxy implementation > From: [email protected] > To: [email protected] > > Hi, > > I have been consuming the proxy bundle with the weaving support for a while > now, however it results in weaving classes that will never be proxies ever. > In fact I want to "weave" the classes that live in applications, but not my > runtime. I would like to be able to disable weaving for bundles in my > runtime. > > I have prototyped a change locally which involves providing a new service: > > package org.apache.aries.proxy.weaving; > > > import org.osgi.framework.Bundle; > > > /** > > * Services of this interface are used by the ProxyManager's weaving > implementation to > > * decide if a specific bundle should be subject to weaving. > > */ > > public interface ProxyWeavingController > > { > > /** > > * Returns true if the bundle should be subject to proxy weaving. If it > returns > > * false then the bundle will not be weaved. The result of this method is > immutable > > * for a given bundle. That means repeated calls given the same bundle > MUST > > * return the same response. > > * > > * @param b the bundle that is being weaved > > * @return true if it should be woven, false otherwise. > > */ > > public boolean shouldWeave(Bundle b); > > } > > I've updated the proxy weaving support to call services and only if they > all agree that the bundle should be woven will it be woven. I'd like to > commit this to the codebase, but I wanted to get peoples thoughts before I > did. > > Thanks > Alasdair > > -- > Alasdair Nottingham > [email protected]
