Fixing the thread safety issue is not too hard - it just involves moving the fields that change state from the model classes to the rendering classes.
I had suggested removing model setXxx methods a while ago and there was some disagreement about that. -Adrian --- On Wed, 5/26/10, Harmeet Bedi <[email protected]> wrote: > From: Harmeet Bedi <[email protected]> > Subject: Re: Developers Note: Screen Widget Model Classes > To: [email protected] > Date: Wednesday, May 26, 2010, 2:11 AM > Wondering if these 2 patterns could > make sense: > > - Pass clone of ModelFormField instance to renderer and > have each instanceof ModelFormField implement Cloneable > contract. Could provide a systematic way to improve > thread safety > Wondering if that would be too expensive. 2 things that may > help: > FlexibleStringExpander instances or any other immutable > references could be shared between cloned and 'this'. > FlexibleStringExpander can be assumed immutable. > Incremental gc in java could mitigate overhead. > > - Restrict access or remove setXXX methods in > ModelFormField instances. > > Had similar problems a while back when i extended > renderers/xsd in our code. So feel the pain. Workaround was > to use only context reference to build pipeline if state > needs to be carried and not change the object. > > Harmeet > > On 19/05/10 1:49 PM, Adrian Crum wrote: > > Just a friendly reminder to the OFBiz developers and > committers: The > > screen widget model classes should not have their > state altered by > > renderers. That is not thread-safe and it results in > undesirable behavior. > > > > I know it's easy to forget - I just deprecated some > methods that I wrote > > where I made that mistake. :-) > > > > It might help to think of the widget model classes as > being immutable > > (though technically they aren't). Once they are > constructed and put in > > the cache, they shouldn't be changed. > > > > If you see code where a renderer calls a model's > renderXxxx method, and > > inside that method the model's fields are being > changed, then that is a > > no-no. > > > > -Adrian > > > >
