>>>Tbis is hard to debug and write tests for, but this is really obvious from looking at the code. And I already had this problem >>>in my apps. It's not funny to randomly pickup instances from other sessions...
This is not a case that our code is not correct! My point is that we have to proof that current code is not working correctly! 2010/4/13 Mark Struberg <[email protected]> > This makes no difference, really. > The problem is that we store a CreationalContext as a member in > NormalScopedBeanInterceptorHandler. And subsequent function calls rely on > it. But if in the meantime _another_ thread overwrites this > NormalScopedBeanInterceptorHandler.creationalContext (by using the same > proxy instance), then we get into troubles. > > Tbis is hard to debug and write tests for, but this is really obvious from > looking at the code. And I already had this problem in my apps. It's not > funny to randomly pickup instances from other sessions... > > LieGrue, > strub > > --- Gurkan Erdogdu <[email protected]> schrieb am Mo, 12.4.2010: > > > Von: Gurkan Erdogdu <[email protected]> > > Betreff: Re: svn commit: r933348 - in > /openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept: > ApplicationScopedBeanIntereptorHandler.java > DependentScopedBeanInterceptorHandler.java InterceptorHandler.java > NormalScopedBeanInterceptorHand > > An: [email protected] > > Datum: Montag, 12. April, 2010 22:29 Uhr > > Please look at AbstractContext, > > creationalContextMap#putIfAbsent call > > > > 2010/4/12 Mark Struberg <[email protected]> > > > > > Sorry, I think we need to rollback the rollback, since > > this re-introduces > > > really heavy concurrency problems. > > > > > > @SessionScoped public class User [ > > > public String getEmail().. > > > } > > > > > > @ApplicationScoped class MailService { > > > private @Inject User usr; > > > > > > public void sendMail(String content) { > > > sendInternal(user.getEmail(), content); > > > } > > > } > > > > > > now, with this change whenever 2 threads (from 2 > > sessions of a webserver) > > > invoke sendMail and hit the user.getEmail() the > > > NormalScopedBeanInterceptorHandler gets called and > > those 2 threads override > > > each other the > > > private transient > > WeakReference<CreationalContext<?>> > > creationalContext = > > > null; > > > because there is only one single proxy instance being > > used by those 2 > > > threads. > > > > > > Got me? > > > > > > LieGrue, > > > strub > > > > > > --- [email protected] > > <[email protected]> > > schrieb am Mo, 12.4.2010: > > > > > > > Von: [email protected] > > <[email protected]> > > > > Betreff: svn commit: r933348 - in > > > > > > /openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept: > > > ApplicationScopedBeanIntereptorHandler.java > > > DependentScopedBeanInterceptorHandler.java > > InterceptorHandler.java > > > NormalScopedBeanInterceptorHandler.java > > > > An: [email protected] > > > > Datum: Montag, 12. April, 2010 20:20 Uhr > > > > Author: gerdogdu > > > > Date: Mon Apr 12 18:20:06 2010 > > > > New Revision: 933348 > > > > > > > > URL: http://svn.apache.org/viewvc?rev=933348&view=rev > > > > Log: > > > > [OWB-351] OWB picks up @SessionScoped contextual > > instances > > > > from expired sessions. Corrects eating client > > provided > > > > creational context via BeanManager#getReference > > > > > > > > Modified: > > > > > > > > > > > > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/ApplicationScopedBeanIntereptorHandler.java > > > > > > > > > > > > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/DependentScopedBeanInterceptorHandler.java > > > > > > > > > > > > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/InterceptorHandler.java > > > > > > > > > > > > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/NormalScopedBeanInterceptorHandler.java > > > > > > > > Modified: > > > > > > > > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/ApplicationScopedBeanIntereptorHandler.java > > > > URL: > > > > http://svn.apache.org/viewvc/openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/ApplicationScopedBeanIntereptorHandler.java?rev=933348&r1=933347&r2=933348&view=diff > > > > > > > > > > ============================================================================== > > > > --- > > > > > > > > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/ApplicationScopedBeanIntereptorHandler.java > > > > (original) > > > > +++ > > > > > > > > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/ApplicationScopedBeanIntereptorHandler.java > > > > Mon Apr 12 18:20:06 2010 > > > > @@ -21,7 +21,6 @@ package > > org.apache.webbeans.intercept; > > > > import > > javax.enterprise.context.spi.CreationalContext; > > > > > > > > import > > org.apache.webbeans.component.OwbBean; > > > > -import > > > > > > org.apache.webbeans.context.creational.CreationalContextImpl; > > > > > > > > > > > > /** > > > > @@ -43,7 +42,7 @@ public class > > ApplicationScopedBeanIntere > > > > /** > > > > * Creates a new > > handler. > > > > * @param bean > > bean > > > > - * @param > > creationalContext > > > > creational context > > > > + * @param > > creationalContext > > > > creaitonal context > > > > */ > > > > public > > > > > > ApplicationScopedBeanIntereptorHandler(OwbBean<?> > > > > bean, CreationalContext<?> > > creationalContext) > > > > { > > > > @@ -53,11 +52,11 @@ public class > > > > ApplicationScopedBeanIntere > > > > /** > > > > * {...@inheritdoc} > > > > */ > > > > - protected Object > > > > getContextualInstance(OwbBean<Object> > > bean, > > > > CreationalContextImpl<?> > > creationalContext) > > > > + protected Object > > > > getContextualInstance(OwbBean<Object> > > bean) > > > > { > > > > if > > (cachedInstance > > > > == null) > > > > { > > > > - > > cachedInstance = > > > > super.getContextualInstance(bean, > > creationalContext); > > > > + > > cachedInstance = > > > > super.getContextualInstance(bean); > > > > } > > > > > > > > return > > > > cachedInstance; > > > > > > > > Modified: > > > > > > > > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/DependentScopedBeanInterceptorHandler.java > > > > URL: > > > > http://svn.apache.org/viewvc/openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/DependentScopedBeanInterceptorHandler.java?rev=933348&r1=933347&r2=933348&view=diff > > > > > > > > > > ============================================================================== > > > > --- > > > > > > > > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/DependentScopedBeanInterceptorHandler.java > > > > (original) > > > > +++ > > > > > > > > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/DependentScopedBeanInterceptorHandler.java > > > > Mon Apr 12 18:20:06 2010 > > > > @@ -67,12 +67,10 @@ public class > > > > DependentScopedBeanIntercep > > > > /** > > > > * {...@inheritdoc} > > > > */ > > > > - protected Object > > callAroundInvokes(Method > > > > proceed, Object[] arguments, > > List<InterceptorData> > > > > stack, > > > > - > > > > > > > > > > > > CreationalContextImpl<?> cc) > > > > - throws Exception > > > > + protected Object > > callAroundInvokes(Method > > > > proceed, Object[] arguments, > > List<InterceptorData> > > > > stack) throws Exception > > > > { > > > > > > > > InvocationContextImpl impl = new > > > > InvocationContextImpl(this.bean, > > this.actualInstance > > > > ,proceed, arguments, stack, > > InterceptorType.AROUND_INVOKE); > > > > - > > > > impl.setCreationalContext(cc); > > > > + > > > > impl.setCreationalContext(creationalContext); > > > > > > > > return > > > > impl.proceed(); > > > > } > > > > > > > > Modified: > > > > > > > > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/InterceptorHandler.java > > > > URL: > > > > http://svn.apache.org/viewvc/openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/InterceptorHandler.java?rev=933348&r1=933347&r2=933348&view=diff > > > > > > > > > > ============================================================================== > > > > --- > > > > > > > > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/InterceptorHandler.java > > > > (original) > > > > +++ > > > > > > > > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/InterceptorHandler.java > > > > Mon Apr 12 18:20:06 2010 > > > > @@ -169,13 +169,11 @@ public abstract class > > > > InterceptorHandler > > > > * @param method > > business method > > > > * @param proceed > > proceed method > > > > * @param arguments > > method arguments > > > > - * @param > > creationalContext bean > > > > creational context > > > > + * @param > > ownerCreationalContext > > > > bean creational context > > > > * @return method > > result > > > > * @throws > > Exception for exception > > > > */ > > > > - public Object invoke(Object > > instance, Method > > > > method, Method proceed, Object[] arguments, > > > > - > > > > > > > > CreationalContextImpl<?> > > > > creationalContext) > > > > - throws Exception > > > > + public Object invoke(Object > > instance, Method > > > > method, Method proceed, Object[] arguments, > > > > CreationalContextImpl<?> > > ownerCreationalContext) > > > > throws Exception > > > > { > > > > //Result of > > > > invocation > > > > Object result > > = > > > > null; > > > > @@ -212,7 +210,7 @@ public abstract class > > > > InterceptorHandler > > > > > > > > > > > > > > ((ProxyObject)delegate).setHandler(this.delegateHandler); > > > > > > > > > > > > // Gets > > component > > > > decorator stack > > > > - > > > > decorators > > = > > > > > > WebBeansDecoratorConfig.getDecoratorStack(injectionTarget, > > > > instance, delegate, creationalContext); > > > > > > > > > > > > + > > > > decorators > > = > > > > > > WebBeansDecoratorConfig.getDecoratorStack(injectionTarget, > > > > instance, delegate, ownerCreationalContext); > > > > > > > > > > > > > > > > //Sets > > decorator > > > > stack of delegate > > > > > > > > > > > > > > this.delegateHandler.setDecorators(decorators); > > > > > > > > > > > > @@ -249,10 +247,7 @@ public abstract class > > > > InterceptorHandler > > > > > > > > // Call Around > > > > Invokes > > > > > > > > if > > > > > > > > > > (WebBeansUtil.isContainsInterceptorMethod(this.interceptedMethodMap.get(method), > > > > InterceptorType.AROUND_INVOKE)) > > > > > > > > { > > > > - > > > > > > return > > > > callAroundInvokes(method, arguments, > > > > - > > > > > > > > > > > > > > > > > > > > > > InterceptorUtil.getInterceptorMethods(this.interceptedMethodMap.get(method), > > > > - > > > > > > > > > > > > > > > > > > > > > > > > InterceptorType.AROUND_INVOKE), > > > > - > > > > > > > > > > > > creationalContext); > > > > + > > > > > > return > > > > callAroundInvokes(method, arguments, > > > > > > > > > > InterceptorUtil.getInterceptorMethods(this.interceptedMethodMap.get(method), > > > > InterceptorType.AROUND_INVOKE)); > > > > > > > > } > > > > > > > > > > > > > > > > } > > > > @@ -307,9 +302,7 @@ public abstract class > > > > InterceptorHandler > > > > * @return return > > of method > > > > * @throws > > Exception for any exception > > > > */ > > > > - protected abstract Object > > > > callAroundInvokes(Method interceptedMethod, > > Object[] > > > > arguments, > > > > - > > > > > > > > > > > > List<InterceptorData> stack, > > > > CreationalContextImpl<?> > > creationalContext) > > > > - throws Exception; > > > > + protected abstract Object > > > > callAroundInvokes(Method interceptedMethod, > > Object[] > > > > arguments, List<InterceptorData> stack) > > throws > > > > Exception; > > > > > > > > /** > > > > * > > > > > > > > Modified: > > > > > > > > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/NormalScopedBeanInterceptorHandler.java > > > > URL: > > > > http://svn.apache.org/viewvc/openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/NormalScopedBeanInterceptorHandler.java?rev=933348&r1=933347&r2=933348&view=diff > > > > > > > > > > ============================================================================== > > > > --- > > > > > > > > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/NormalScopedBeanInterceptorHandler.java > > > > (original) > > > > +++ > > > > > > > > > > openwebbeans/trunk/webbeans-impl/src/main/java/org/apache/webbeans/intercept/NormalScopedBeanInterceptorHandler.java > > > > Mon Apr 12 18:20:06 2010 > > > > @@ -18,6 +18,7 @@ > > > > */ > > > > package org.apache.webbeans.intercept; > > > > > > > > +import java.lang.ref.WeakReference; > > > > import java.lang.reflect.Method; > > > > import java.util.List; > > > > > > > > @@ -42,6 +43,9 @@ public class > > NormalScopedBeanInterceptor > > > > /**Serial id*/ > > > > private static final long > > > > serialVersionUID = 1L; > > > > > > > > + /**Creational context*/ > > > > + private transient > > > > WeakReference<CreationalContext<?>> > > > > creationalContext = null; > > > > + > > > > /** > > > > * Creates a new > > bean instance > > > > * @param bean > > bean > > > > @@ -50,6 +54,7 @@ public class > > NormalScopedBeanInterceptor > > > > public > > > > > > NormalScopedBeanInterceptorHandler(OwbBean<?> bean, > > > > CreationalContext<?> creationalContext) > > > > { > > > > super(bean); > > > > + > > this.creationalContext = new > > > > > > WeakReference<CreationalContext<?>>(creationalContext); > > > > } > > > > > > > > /** > > > > @@ -58,38 +63,21 @@ public class > > > > NormalScopedBeanInterceptor > > > > @Override > > > > public Object invoke(Object > > > > instance, Method method, Method proceed, Object[] > > arguments) > > > > throws Exception > > > > { > > > > - > > CreationalContextImpl<?> > > > > creationalContext = null; > > > > - > > > > - //Context of the > > bean > > > > - Context > > webbeansContext = > > > > getBeanManager().getContext(bean.getScope()); > > > > - > > > > - if (webbeansContext > > instanceof > > > > AbstractContext) > > > > - { > > > > - > > > > creationalContext = > > (CreationalContextImpl<?>) > > > > > > ((AbstractContext)webbeansContext).getCreationalContext(bean); > > > > - } > > > > - if > > (creationalContext == > > > > null) > > > > - { > > > > - // if > > there was > > > > no CreationalContext set from external, we create > > a new one > > > > - > > > > creationalContext = > > (CreationalContextImpl<?>) > > > > > > CreationalContextFactory.getInstance().getCreationalContext(bean); > > > > - } > > > > - > > > > //Get instance > > from > > > > context > > > > - Object > > webbeansInstance = > > > > getContextualInstance((OwbBean<Object>) > > this.bean, > > > > creationalContext); > > > > + Object > > webbeansInstance = > > > > getContextualInstance((OwbBean<Object>) > > this.bean); > > > > > > > > //Call super > > > > - return > > > > super.invoke(webbeansInstance, method, proceed, > > arguments, > > > > creationalContext); > > > > + return > > > > super.invoke(webbeansInstance, method, proceed, > > arguments, > > > > (CreationalContextImpl<?>) > > > > this.creationalContext.get()); > > > > } > > > > > > > > /** > > > > * {...@inheritdoc} > > > > */ > > > > - protected Object > > callAroundInvokes(Method > > > > proceed, Object[] arguments, > > List<InterceptorData> > > > > stack, CreationalContextImpl<?> > > creationalContext) > > > > - throws Exception > > > > + protected Object > > callAroundInvokes(Method > > > > proceed, Object[] arguments, > > List<InterceptorData> > > > > stack) throws Exception > > > > { > > > > - > > InvocationContextImpl impl = > > > > new InvocationContextImpl(this.bean, > > > > - > > > > > > > > > > > > > > > > > > getContextualInstance((OwbBean<Object>) > > > > this.bean, creationalContext), > > > > + > > InvocationContextImpl impl = > > > > new InvocationContextImpl(this.bean, > > > > getContextualInstance((OwbBean<Object>) > > this.bean), > > > > > > > > > > > > > > > > > > > > proceed, arguments, stack, > > InterceptorType.AROUND_INVOKE); > > > > - > > > > impl.setCreationalContext(creationalContext); > > > > + > > > > > > impl.setCreationalContext(creationalContext.get()); > > > > > > > > return > > > > impl.proceed(); > > > > > > > > @@ -99,10 +87,9 @@ public class > > > > NormalScopedBeanInterceptor > > > > /** > > > > * Gets instance > > from context. > > > > * @param bean bean > > instance > > > > - * @param > > creationalContext > > > > * @return the > > underlying contextual > > > > instance, either cached or resolved from the > > context > > > > */ > > > > - protected Object > > > > getContextualInstance(OwbBean<Object> > > bean, > > > > CreationalContextImpl<?> > > creationalContext) > > > > + protected Object > > > > getContextualInstance(OwbBean<Object> > > bean) > > > > { > > > > Object > > > > webbeansInstance = null; > > > > > > > > @@ -117,8 +104,22 @@ public class > > > > NormalScopedBeanInterceptor > > > > > > return > > > > webbeansInstance; > > > > } > > > > > > > > + if (webbeansContext > > instanceof > > > > AbstractContext) > > > > + { > > > > + > > > > CreationalContext<?> cc = > > > > > > ((AbstractContext)webbeansContext).getCreationalContext(bean); > > > > + if (cc > > != null) > > > > + { > > > > + > > > > creationalContext = new > > > > > > WeakReference<CreationalContext<?>>(cc); > > > > + } > > > > + } > > > > + if > > (creationalContext == > > > > null) > > > > + { > > > > + // if > > there was > > > > no CreationalContext set from external, we create > > a new one > > > > + > > > > creationalContext = new > > > > > > > > > > WeakReference<CreationalContext<?>>(CreationalContextFactory.getInstance().getCreationalContext(bean)); > > > > + } > > > > + > > > > // finally, we > > > > create a new contextual instance > > > > - webbeansInstance = > > > > > > webbeansContext.get((Contextual<Object>)this.bean, > > > > (CreationalContext<Object>) > > creationalContext); > > > > + webbeansInstance = > > > > > > webbeansContext.get((Contextual<Object>)this.bean, > > > > (CreationalContext<Object>) > > creationalContext.get()); > > > > > > > > return > > > > webbeansInstance; > > > > } > > > > > > > > > > > > > > > > > > __________________________________________________ > > > Do You Yahoo!? > > > Sie sind Spam leid? Yahoo! Mail verfügt über einen > > herausragenden Schutz > > > gegen Massenmails. > > > http://mail.yahoo.com > > > > > > > > > > > -- > > Gurkan Erdogdu > > http://gurkanerdogdu.blogspot.com > > > > > __________________________________________________ > Do You Yahoo!? > Sie sind Spam leid? Yahoo! Mail verfügt über einen herausragenden Schutz > gegen Massenmails. > http://mail.yahoo.com > -- Gurkan Erdogdu http://gurkanerdogdu.blogspot.com
