Bug 52957 - ClassCastException while removing Valve
Summary: ClassCastException while removing Valve
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 7.0.26
Hardware: All All
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-21 12:47 UTC by Violeta Georgieva
Modified: 2012-03-22 19:10 UTC (History)
0 users



Attachments
Patch proposal (616 bytes, patch)
2012-03-21 12:47 UTC, Violeta Georgieva
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Violeta Georgieva 2012-03-21 12:47:49 UTC
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
Comment 1 Konstantin Kolinko 2012-03-21 12:59:40 UTC
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
Comment 2 Mark Thomas 2012-03-21 13:09:33 UTC
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.
Comment 3 Mark Thomas 2012-03-22 14:40:07 UTC
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.
Comment 4 Violeta Georgieva 2012-03-22 19:10:57 UTC
Thanks