rom: "Giacomo Pati" <[EMAIL PROTECTED]>
> On Thu, 12 Dec 2002, Konstantin Piroumian wrote:
> > From: "Giacomo Pati" <[EMAIL PROTECTED]>
> >
> > >
> > > Hi team
> > >
> > > We encountered some thread race issues with the LocalAction class.
> > >
> > > 1. The class is marked ThreadSafe, so there will be only one instance
in
> > > the system.
> >
> > Yup.
> >
> > >
> > > 2. There are member variables (langAttr, localeAttr, etc.) that are
set by
> > > the checkParams method on each invocation but only with loglevel
DEBUG,
> > > and thus any parameter passed in at runtime will be ignored at any
other
> > > log level setting. After consulting CVS the checkParam method was put
into
> > > a isDebugEnabled-block 10 month ago. I wonder why nobody ever had this
> > > issue before.
> >
> > That was done intentionally, mainly for performance reasones and because
of
> > the thread safety problems you are faced now. It was never intended to
allow
> > to set those parameters on pipeline level.
>
> Ok. Does anybody has any objections if I fix it to be able to set it at
> pipeline level? This might be usefull on sites which allows you to change
> language on the fly.
Hm. Seems that either you are going to implement something that is already
done in LocaleAction or I don't quite understand your needs. Let me clarify
the action's behavior: all those member variables are really only for
configuration purposes, they don't have much meaning at the pipeline level.
See my detailed comments below:
private void checkParams(Parameters par) {
// The code below sets the names of sitemap variables that will be
// returned after the actions is called: lang, country, variant,
locale (defaults).
langAttr = par.getParameter(LANG_ATTR, langAttr);
countryAttr = par.getParameter(COUNTRY_ATTR, countryAttr);
variantAttr = par.getParameter(VARIANT_ATTR, variantAttr);
localeAttr = par.getParameter(LOCALE_ATTR, localeAttr);
// Indicates whether to store the locale in a request attribute
storeInRequest = par.getParameterAsBoolean(STORE_REQUEST,
storeInRequest);
// Indicates whether to store the locale in a session attribute
storeInSession = par.getParameterAsBoolean(STORE_SESSION,
storeInSession);
// Indicates whether to create a session to store the locale if
required
createSession = par.getParameterAsBoolean(CREATE_SESSION,
createSession);
// Indicates whether to save locale in a cookie, to be retained at
the client side
storeInCookie = par.getParameterAsBoolean(STORE_COOKIE,
storeInCookie);
The real locale selection is performed at lines 276 - 277:
// obtain locale information from params, session or cookies
String lc = getLocaleAttribute(objectModel, localeAttr);
Locale locale = I18nUtils.parseLocale(lc);
If take a look at i18n samples then you'll see how LocaleAction is used to
change the language on fly. This does not require changing the returning
sitemap variable names for this, neither changing locale retaining settings.
If you really need to change all those things at the pipeline level then I
have no objections. But do you really need it? Would you please provide a
simlpe use-case?
Konstantin
>
> > > 3. Because of 2. two concurrent threads using LocalAction will
overwrite
> > > each others settings which will confuse users (given it runs with
DEBUG
> > > level).
> >
> > I suppose that DEBUG level is used only for debug purposes and never in
> > production environments, so the problem is not much critical. But I have
no
> > objections if that will be fixed somehow, though, I don't think that
making
> > the action Poolable is a good idea.
>
> Absolutely. I intend to move the member variables into the act method as
> well as most of the statements from checkParams.
>
> > > OT: The usage of the debug method is a *big NO* for performance
reasons.
>
> I'll fix that too ;)
>
> Giacomo
>
> > > This issue is true for both branches (HEAD and cocoon_2_0_3_branch)
even
> > > if that class is different on each branch.
> > >
> > > Is anybody working on that class already?
> >
> > I think no.
> > I'd prefer to replace it by an Input module, which is much nicer in the
> > sitemap, that will allow to _get_ locale parameters, though, I have no
idea
> > yet on how to _set_ the locale parameters.
> >
> > Regards,
> > Konstantin
> >
> > >
> > > Giacomo
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, email: [EMAIL PROTECTED]
