You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Giacomo Pati <gi...@apache.org> on 2002/12/12 13:50:46 UTC
Issues with LocalAction
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.
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.
3. Because of 2. two concurrent threads using LocalAction will overwrite
each others settings which will confuse users (given it runs with DEBUG
level).
OT: The usage of the debug method is a *big NO* for performance reasons.
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?
Giacomo
---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org
Re: Issues with LocalAction
Posted by Konstantin Piroumian <kp...@apache.org>.
rom: "Giacomo Pati" <gi...@apache.org>
> On Thu, 12 Dec 2002, Konstantin Piroumian wrote:
> > From: "Giacomo Pati" <gi...@apache.org>
> >
> > >
> > > 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: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org
Re: Issues with LocalAction
Posted by Giacomo Pati <gi...@apache.org>.
On Thu, 12 Dec 2002, Konstantin Piroumian wrote:
> From: "Giacomo Pati" <gi...@apache.org>
>
> >
> > 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.
> > 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: cocoon-dev-unsubscribe@xml.apache.org
> > For additional commands, email: cocoon-dev-help@xml.apache.org
> >
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
> For additional commands, email: cocoon-dev-help@xml.apache.org
>
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org
Re: Issues with LocalAction
Posted by Konstantin Piroumian <kp...@apache.org>.
From: "Giacomo Pati" <gi...@apache.org>
>
> 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.
>
> 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.
>
> OT: The usage of the debug method is a *big NO* for performance reasons.
>
> 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: cocoon-dev-unsubscribe@xml.apache.org
> For additional commands, email: cocoon-dev-help@xml.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org