You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openwebbeans.apache.org by Mark Struberg <st...@yahoo.de.INVALID> on 2021/05/18 05:19:07 UTC
RequestContextController activate/deactivate
hi folks!
I'm not really happy with the RequestContextController. And it appears to me that the spec is broken.
Here is the important part from the spec:
6.5.2. Activating Built In Contexts
Certain built in contexts support the ability to be activated and deactivated. This allows developers to control built-in contexts in ways that they could also manage custom built contexts.
When activating and deactivating built in contexts, it is important to realize that they can only be activated if not already active within a given thread.
6.5.2.1. Activating a Request Context
Request contexts can be managed either programmatically or via interceptor.
To programmatically manage request contexts, the container provides a built in bean that is @Dependent scoped and of type RequestContextController that allows you to activate and deactivate a request context on the current thread. The object should be considered stateful, invoking the same instance on different threads may not work properly, non-portable behavior may occur.
public interface RequestContextController {
boolean activate();
void deactivate() throws ContextNotActiveException;
}
When the activate() method is called, if the request context is not already active on the current thread then it will be activated and the method returns true. Otherwise, the method returns false.
When the deactivate() method is called, if this controller started the request context then the request context is stopped. The method does nothing if this controller did not activate the context
The problem is that there are 2 ways to use that part
a.)
boolean didActivate = reqCtxCtrl.activate();
...
if (didActivate) reqCtxCrl.deactivate();
b.)
try {
reqCtxCtrl.activate();
...
} finally {
reqCtxCrl.deactivate();
}
The problematic part is nesting.
In case a) we got maybe 7 calls to activate() but only 1 to deactivate.
In case b) we got 7 calls to activate and 7 to deactivate();
There is simply no way to implement this in a clean way. My original impl was of course wrong. The ThreadLocal<List> does not help much either. If we use a ThreadLocal<Boolean> we potentially leak memory in case of a). If we close immediately we potentially close way too early in case b).
Wdyt?
Gonna open a ticket in the spec.
LieGrue,
strub
Re: RequestContextController activate/deactivate
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Hi
Le mar. 18 mai 2021 à 08:41, Benjamin Marwell <bm...@apache.org> a
écrit :
> Hi Mark,
>
> for case a), would an AtomicInteger work as well (instead of boolean
> didActivate)? This way you could deactivate for each activate() call.
>
Does not work, issue is you can have open()/close() pattern or open() only
if it returns false.
Means you can count but don't know what to do in close since you have to
skip some calls so you need this counter + boolean so at the end a list of
boolean (current impl except it uses an enum but it is equivalent).
Now the issue is that current impl assumes open/close are symmetric whereas
the spec enables them to be or not.
Typiclaly using built in wrapper interceptor will not be symmetric:
https://github.com/apache/openwebbeans/blob/master/webbeans-impl/src/main/java/org/apache/webbeans/context/control/ActivateRequestContextInterceptorBean.java#L78
So in this case we can leak as mark explained.
But in case the app uses a symmetric pattern (not rare) it works and fixing
previous case breaks our interceptor.
Broken spec it seems :(
>
> Am Di., 18. Mai 2021 um 07:19 Uhr schrieb Mark Struberg
> <st...@yahoo.de.invalid>:
> >
> > hi folks!
> >
> > I'm not really happy with the RequestContextController. And it appears
> to me that the spec is broken.
> >
> > Here is the important part from the spec:
> >
> > 6.5.2. Activating Built In Contexts
> > Certain built in contexts support the ability to be activated and
> deactivated. This allows developers to control built-in contexts in ways
> that they could also manage custom built contexts.
> > When activating and deactivating built in contexts, it is important to
> realize that they can only be activated if not already active within a
> given thread.
> > 6.5.2.1. Activating a Request Context
> > Request contexts can be managed either programmatically or via
> interceptor.
> > To programmatically manage request contexts, the container provides a
> built in bean that is @Dependent scoped and of type
> RequestContextController that allows you to activate and deactivate a
> request context on the current thread. The object should be considered
> stateful, invoking the same instance on different threads may not work
> properly, non-portable behavior may occur.
> > public interface RequestContextController {
> > boolean activate();
> > void deactivate() throws ContextNotActiveException;
> > }
> > When the activate() method is called, if the request context is not
> already active on the current thread then it will be activated and the
> method returns true. Otherwise, the method returns false.
> > When the deactivate() method is called, if this controller started the
> request context then the request context is stopped. The method does
> nothing if this controller did not activate the context
> >
> > The problem is that there are 2 ways to use that part
> >
> > a.)
> > boolean didActivate = reqCtxCtrl.activate();
> > ...
> > if (didActivate) reqCtxCrl.deactivate();
> >
> > b.)
> > try {
> > reqCtxCtrl.activate();
> > ...
> > } finally {
> > reqCtxCrl.deactivate();
> > }
> >
> > The problematic part is nesting.
> > In case a) we got maybe 7 calls to activate() but only 1 to deactivate.
> > In case b) we got 7 calls to activate and 7 to deactivate();
> >
> > There is simply no way to implement this in a clean way. My original
> impl was of course wrong. The ThreadLocal<List> does not help much either.
> If we use a ThreadLocal<Boolean> we potentially leak memory in case of a).
> If we close immediately we potentially close way too early in case b).
> >
> > Wdyt?
> > Gonna open a ticket in the spec.
> >
> > LieGrue,
> > strub
> >
>
Re: RequestContextController activate/deactivate
Posted by Benjamin Marwell <bm...@apache.org>.
Hi Mark,
for case a), would an AtomicInteger work as well (instead of boolean
didActivate)? This way you could deactivate for each activate() call.
Am Di., 18. Mai 2021 um 07:19 Uhr schrieb Mark Struberg
<st...@yahoo.de.invalid>:
>
> hi folks!
>
> I'm not really happy with the RequestContextController. And it appears to me that the spec is broken.
>
> Here is the important part from the spec:
>
> 6.5.2. Activating Built In Contexts
> Certain built in contexts support the ability to be activated and deactivated. This allows developers to control built-in contexts in ways that they could also manage custom built contexts.
> When activating and deactivating built in contexts, it is important to realize that they can only be activated if not already active within a given thread.
> 6.5.2.1. Activating a Request Context
> Request contexts can be managed either programmatically or via interceptor.
> To programmatically manage request contexts, the container provides a built in bean that is @Dependent scoped and of type RequestContextController that allows you to activate and deactivate a request context on the current thread. The object should be considered stateful, invoking the same instance on different threads may not work properly, non-portable behavior may occur.
> public interface RequestContextController {
> boolean activate();
> void deactivate() throws ContextNotActiveException;
> }
> When the activate() method is called, if the request context is not already active on the current thread then it will be activated and the method returns true. Otherwise, the method returns false.
> When the deactivate() method is called, if this controller started the request context then the request context is stopped. The method does nothing if this controller did not activate the context
>
> The problem is that there are 2 ways to use that part
>
> a.)
> boolean didActivate = reqCtxCtrl.activate();
> ...
> if (didActivate) reqCtxCrl.deactivate();
>
> b.)
> try {
> reqCtxCtrl.activate();
> ...
> } finally {
> reqCtxCrl.deactivate();
> }
>
> The problematic part is nesting.
> In case a) we got maybe 7 calls to activate() but only 1 to deactivate.
> In case b) we got 7 calls to activate and 7 to deactivate();
>
> There is simply no way to implement this in a clean way. My original impl was of course wrong. The ThreadLocal<List> does not help much either. If we use a ThreadLocal<Boolean> we potentially leak memory in case of a). If we close immediately we potentially close way too early in case b).
>
> Wdyt?
> Gonna open a ticket in the spec.
>
> LieGrue,
> strub
>