On 11/29/05, Rahul Akolkar <[EMAIL PROTECTED]> wrote: > > On 11/15/05, Craig McClanahan <[EMAIL PROTECTED]> wrote: > > On 11/14/05, Rahul Akolkar <[EMAIL PROTECTED]> wrote: > <snip/> > > > > > > The other major item its complaining about is non-transient > > > non-serializable fields in Serializable classes. That explains some of > > > the NotSerializableException's I get with sticky sessions on Tomcat > > > startup (mostly in subclasses of Commons Chain's base impls). Will > > > take a stab at that soon. > > > > > > Thanks ... those are important. Not just because we need to honor the > > contracts about Serializable session scope attributes, but they also > > indicate potential architectural issues if we are maintaining references > to > > non-Serializable stuff in session scope that should really not be > > referenced. Struts 1.x has suffered from some of this kind of thing, > causing > > memory bloat due to duplicated object trees when you migrate a session > from > > one instance to another. > > > <snap/> > > I got around to looking at some code today in serializability's light, > and that leads to a few questions. I bring this up because I've err'ed > here before and I wouldn't want [shale] (or [resources] - a similar > discussion is probably in order on commons-dev) to do the same.
In general, here's the policy that I propose to follow w.r.t. serialization: * Any class for which an instance might get stored (by the framework, or by the application with the blessing of framework documentation that says to do so) must be serializable. * Any class that is not serializable should not claim that it "implements Serializable". That being said, let's review the cases: First, the offenders include: > > [shale] > * ServletRemoteContext Only used in the context of a single request. Unfortunately, there is no way to say "I'm not Serializabl even though my parent class claims that I am" ... as is the case here, inherited from Commons chain. * CommonsValidatorTag Tag instances are used only in the context of a particular request -- no reason to worry about serializability. * CommonsValidator This is a JSF Validator, so it could well be attached to a UIComponent stored in session scope. It's not obvious from a static review of the code what's the problem, but if there is one it'll need to be fixed. > * ShalePhaseListener Never stored in session scope, so doesn't need to be serializable. * ShaleWebContext Only used in the context of a single request. Same "inherited claim" issue as ServletRemoteContext above. * StatusImpl Absolutely has to be serializable -- it's probably the non-static non-transient Log instance here. [clay] > * ClayContext > * ComponentBean > * BuilderRuleContext > * SymbolTag I don't *think* any of these would ever need to be stored in session scope, but Gary will need to confirm that. Some might be serializable, others are clearly not, even with any lazy > initialization attempts. So, for each, maybe its best to revisit what > prompted the serializability requirement, and how much each needs to > be pushed. > > My cursory observations: > * For the *Tag impls there isn't much leeway, so it might be worth > biting the bullet > * StatusImpl needs to be serializable by design > * ShaleWebContext is in a tough spot already (see below) > > What are the Shale developers' views on the rest? See above. It might not be a bad idea to have test cases to show that the round > trips work as advertised (I can volunteer some), once this gets sorted > out. +1. In similar scenarios for "day job" projects, we test serializability by JUnit test cases that try to serialize and then deserialize instances that have been configured with various combinations of properties. Not foolproof, but certainly catches the obvious cases. > For the cases where this is really issues with Commons Chain > implementation > > classes, we will (of course) need to supply patches for that. > Fortunately, > > I'm a committer there too :-). > > > <snip/> > > Yes, having looked at the developer list for [chain], let me ask one > [OT] question here, and I'll move to commons-dev (or user) if it > becomes apparent this deserves its own thread: > > Digging into [chain], I found a related commit message on top of > ContextBase [1]. Were there any further developments on that? Seems > like it may be required to allow subclass implementors (of the base > impls in chain) to make their own decisions about serializability to > liberate the *WebContext classes, but that probably won't fit a minor > release (I'm not even sure if one is planned for [chain]). It's not obvious what we should do here ... but it's also not a decision we (Shale developers) or even we (Struts developers) can make in a vacuum. Needs to be addressed on commons-dev so any other stakeholders who care can pitch in an opinion. My other general comment is that right at the moment I want to file an issue against the Shale classes above that we know need to be addressed, then flag it to be resolved LATER (and note in the release notes) so that we can address it after the initial milestone release. -Rahul Craig [1] > http://svn.apache.org/viewcvs.cgi/jakarta/commons/proper/chain/trunk/src/java/org/apache/commons/chain/impl/ContextBase.java > > > > > -Rahul > > > > > > Craig > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > >
