Created attachment 28490 [details] Patch proposal Hello, The exception below is thrown when StandardPipeline.removeValve() is invoked: Caused by: java.lang.ClassCastException: test.MyValve cannot be cast to org.apache.catalina.Lifecycle at org.apache.catalina.core.StandardPipeline.removeValve(StandardPipeline.java:461) at org.apache.catalina.core.StandardPipeline.destroyInternal(StandardPipeline.java:222) at org.apache.catalina.util.LifecycleBase.destroy(LifecycleBase.java:304) test.MyValve implements only Valve I'm attaching a patch made against 7.0.x. Please review it and comment it. Thanks in advance. Regards Violeta Georgieva
One of changes done is Tomcat 7 is that implementing Lifecycle is mandatory in most components. I do not see a reason why a Valve should be an exception. I would suggest you to extends the ValveBase class. http://tomcat.apache.org/migration-7.html#Internal_APIs
If we wanted Valve to extend Lifecycle that then is what we should have done but we didn't. (We did for Container). I'm not against taking a further look at this but I'd want to look more widely at where else we make the assumption Valve implements Lifecycle and the conclusion may well be to change Valve to extend Lifecycle at least in Tomcat 8 if not in 7.
Fixed in trunk and 7.0.x and will be included in 7.0.27 onwards. I went with a marginally more efficient patch. I did look more widely at where Lifecycle is used and it is essentially optional for a wide range of Components (Valve, Realm, Pipeline, Manager, Loader, etc.) I'm not against making it mandatory (neither am I strongly for it) but that is certainly a Tomcat 8 decision. If folks feel strongly about this, the dev list is the place to start the discussion.
Thanks