You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openwebbeans.apache.org by Joseph Bergmark <be...@apache.org> on 2010/04/28 14:18:03 UTC

Re: svn commit: r938812 - /openwebbeans/trunk/webbeans-web/src/main/java/org/apache/webbeans/web/context/WebContextsService.java

Thanks Gurkan!

I'm a little worried about making the Dependent context non-threadlocal and
static and then setting it to null during destroy.  I believe that could
lead to problems in tiered classloading environments with multiple
applications.

For example App1 starts,  App2 starts (referencing the same static Dependent
Context), App 1 stops setting the DependentContext to null and next time
App2 references the ContextService its going to throw a NPE?

Sincerely,

Joe Bergmark

On Wed, Apr 28, 2010 at 3:44 AM, <ge...@apache.org> wrote:

> Author: gerdogdu
> Date: Wed Apr 28 07:44:51 2010
> New Revision: 938812
>
> URL: http://svn.apache.org/viewvc?rev=938812&view=rev
> Log:
> [OWB-363] Intermittent bug with ApplicationScope disposers not being called
>
> Modified:
>
>  openwebbeans/trunk/webbeans-web/src/main/java/org/apache/webbeans/web/context/WebContextsService.java
>
> Modified:
> openwebbeans/trunk/webbeans-web/src/main/java/org/apache/webbeans/web/context/WebContextsService.java
> URL:
> http://svn.apache.org/viewvc/openwebbeans/trunk/webbeans-web/src/main/java/org/apache/webbeans/web/context/WebContextsService.java?rev=938812&r1=938811&r2=938812&view=diff
>
> ==============================================================================
> ---
> openwebbeans/trunk/webbeans-web/src/main/java/org/apache/webbeans/web/context/WebContextsService.java
> (original)
> +++
> openwebbeans/trunk/webbeans-web/src/main/java/org/apache/webbeans/web/context/WebContextsService.java
> Wed Apr 28 07:44:51 2010
> @@ -63,7 +63,7 @@ public class WebContextsService extends
>     private static ThreadLocal<SingletonContext> singletonContext = null;
>
>     /**Current dependent context*/
> -    private static ThreadLocal<DependentContext> dependentContext = null;
> +    private static DependentContext dependentContext;
>
>     /**Current application contexts*/
>     private static Map<ServletContext, ApplicationContext>
> currentApplicationContexts = new ConcurrentHashMap<ServletContext,
> ApplicationContext>();
> @@ -86,8 +86,12 @@ public class WebContextsService extends
>         sessionContext = new ThreadLocal<SessionContext>();
>         applicationContext = new ThreadLocal<ApplicationContext>();
>         conversationContext = new ThreadLocal<ConversationContext>();
> -        dependentContext = new ThreadLocal<DependentContext>();
>         singletonContext = new ThreadLocal<SingletonContext>();
> +
> +        //Dependent context is always active
> +        dependentContext = new DependentContext();
> +        dependentContext.setActive(true);
> +
>     }
>
>     /**
> @@ -114,26 +118,34 @@ public class WebContextsService extends
>     @Override
>     public void destroy(Object destroyObject)
>     {
> -        currentApplicationContexts.clear();
> -        currentSingletonContexts.clear();
> -
> +        //Destroy application context
>         endContext(ApplicationScoped.class, destroyObject);
> +
> +        //Destroy singleton context
>         endContext(Singleton.class, destroyObject);
> +
> +        //Clear saved contexts related with
> +        //this servlet context
> +        currentApplicationContexts.clear();
> +        currentSingletonContexts.clear();
>
> +        //Thread local values to null
>         requestContext.set(null);
> -        dependentContext.set(null);
>         sessionContext.set(null);
>         conversationContext.set(null);
>         applicationContext.set(null);
>         singletonContext.set(null);
>
> +        //Remove thread locals
> +        //for preventing memory leaks
>         requestContext.remove();
> -        dependentContext.remove();
>         sessionContext.remove();
>         conversationContext.remove();
>         applicationContext.remove();
>         singletonContext.remove();
>
> +        //remove dependent context
> +        dependentContext = null;
>     }
>
>
> @@ -165,6 +177,7 @@ public class WebContextsService extends
>             else if(scopeType.equals(Dependent.class))
>             {
>                 //Do nothing
> +                dependentContext = null;
>             }
>             else
>             {
> @@ -200,7 +213,7 @@ public class WebContextsService extends
>             }
>             else if(scopeType.equals(Dependent.class))
>             {
> -                return getDependentContext();
> +                return dependentContext;
>             }
>             else
>             {
> @@ -311,9 +324,8 @@ public class WebContextsService extends
>             {
>                 context.destroy();
>             }
> -
> -            requestContext.set(null);
>
> +            requestContext.set(null);
>         }
>     }
>
> @@ -396,25 +408,35 @@ public class WebContextsService extends
>      */
>     private void destroyApplicationContext(ServletContext servletContext)
>     {
> -        if (applicationContext != null)
> +        //look for thread local
> +        //this can be set by initRequestContext
> +        ApplicationContext context = applicationContext.get();
> +
> +        //no context set
> +        //look for saved application context
> +        if(context == null)
>         {
> -            ApplicationContext context = getApplicationContext();
> -
> -            if (context != null)
> +            if(servletContext != null)
>             {
> -                context.destroy();
> +                context = currentApplicationContexts.get(servletContext);
>             }
> -
> -            applicationContext.set(null);
> -
>         }
>
> +        //Destroy context
> +        if(context != null)
> +        {
> +            context.destroy();
> +        }
> +
> +        //Remove from saved contexts
>         if(servletContext != null)
>         {
>             currentApplicationContexts.remove(servletContext);
>         }
>
> +        //destroy all sessions
>         sessionCtxManager.destroyAllSessions();
> +        //destroy all conversations
>         conversationManager.destroyAllConversations();
>     }
>
> @@ -440,8 +462,7 @@ public class WebContextsService extends
>
>             }
>
> -            singletonContext.set(context);
> -
> +            singletonContext.set(context);
>         }
>
>     }
> @@ -452,19 +473,26 @@ public class WebContextsService extends
>      */
>     private void destroySingletonContext(ServletContext servletContext)
>     {
> -        if (singletonContext != null)
> +        SingletonContext context = getSingletonContext();
> +
> +        //no context set by initRequestContext
> +        if(context == null)
>         {
> -            SingletonContext context = getSingletonContext();
> -
> -            if (context != null)
> +            //look for saved context
> +            if(servletContext != null)
>             {
> -                context.destroy();
> +                context = currentSingletonContexts.get(servletContext);
>             }
> -
> -            singletonContext.set(null);
> -
>         }
>
> +        //context is not null
> +        //destroy it
> +        if(context != null)
> +        {
> +            context.destroy();
> +        }
> +
> +        //remove it from saved contexts
>         if(servletContext != null)
>         {
>             currentSingletonContexts.remove(servletContext);
> @@ -561,22 +589,4 @@ public class WebContextsService extends
>     {
>         return conversationContext.get();
>     }
> -
> -    /**
> -     * Gets dependent context.
> -     * @return dependent context
> -     */
> -    private DependentContext getDependentContext()
> -    {
> -        DependentContext dependentCtx = dependentContext.get();
> -
> -        if (dependentCtx == null)
> -        {
> -            dependentCtx = new DependentContext();
> -            dependentContext.set(dependentCtx);
> -        }
> -
> -        return dependentCtx;
> -    }
> -
>  }
> \ No newline at end of file
>
>
>

Re: svn commit: r938812 - /openwebbeans/trunk/webbeans-web/src/main/java/org/apache/webbeans/web/context/WebContextsService.java

Posted by Gurkan Erdogdu <gu...@yahoo.com>.
Ahh forget about multi-tier :) Will remove null assignnment.



________________________________
From: Joseph Bergmark <be...@apache.org>
To: dev@openwebbeans.apache.org
Sent: Wed, April 28, 2010 3:18:03 PM
Subject: Re: svn commit: r938812 - /openwebbeans/trunk/webbeans-web/src/main/java/org/apache/webbeans/web/context/WebContextsService.java

Thanks Gurkan!

I'm a little worried about making the Dependent context non-threadlocal and
static and then setting it to null during destroy.  I believe that could
lead to problems in tiered classloading environments with multiple
applications.

For example App1 starts,  App2 starts (referencing the same static Dependent
Context), App 1 stops setting the DependentContext to null and next time
App2 references the ContextService its going to throw a NPE?

Sincerely,

Joe Bergmark

On Wed, Apr 28, 2010 at 3:44 AM, <ge...@apache.org> wrote:

> Author: gerdogdu
> Date: Wed Apr 28 07:44:51 2010
> New Revision: 938812
>
> URL: http://svn.apache.org/viewvc?rev=938812&view=rev
> Log:
> [OWB-363] Intermittent bug with ApplicationScope disposers not being called
>
> Modified:
>
>  openwebbeans/trunk/webbeans-web/src/main/java/org/apache/webbeans/web/context/WebContextsService.java
>
> Modified:
> openwebbeans/trunk/webbeans-web/src/main/java/org/apache/webbeans/web/context/WebContextsService.java
> URL:
> http://svn.apache.org/viewvc/openwebbeans/trunk/webbeans-web/src/main/java/org/apache/webbeans/web/context/WebContextsService.java?rev=938812&r1=938811&r2=938812&view=diff
>
> ==============================================================================
> ---
> openwebbeans/trunk/webbeans-web/src/main/java/org/apache/webbeans/web/context/WebContextsService.java
> (original)
> +++
> openwebbeans/trunk/webbeans-web/src/main/java/org/apache/webbeans/web/context/WebContextsService.java
> Wed Apr 28 07:44:51 2010
> @@ -63,7 +63,7 @@ public class WebContextsService extends
>     private static ThreadLocal<SingletonContext> singletonContext = null;
>
>     /**Current dependent context*/
> -    private static ThreadLocal<DependentContext> dependentContext = null;
> +    private static DependentContext dependentContext;
>
>     /**Current application contexts*/
>     private static Map<ServletContext, ApplicationContext>
> currentApplicationContexts = new ConcurrentHashMap<ServletContext,
> ApplicationContext>();
> @@ -86,8 +86,12 @@ public class WebContextsService extends
>         sessionContext = new ThreadLocal<SessionContext>();
>         applicationContext = new ThreadLocal<ApplicationContext>();
>         conversationContext = new ThreadLocal<ConversationContext>();
> -        dependentContext = new ThreadLocal<DependentContext>();
>         singletonContext = new ThreadLocal<SingletonContext>();
> +
> +        //Dependent context is always active
> +        dependentContext = new DependentContext();
> +        dependentContext.setActive(true);
> +
>     }
>
>     /**
> @@ -114,26 +118,34 @@ public class WebContextsService extends
>     @Override
>     public void destroy(Object destroyObject)
>     {
> -        currentApplicationContexts.clear();
> -        currentSingletonContexts.clear();
> -
> +        //Destroy application context
>         endContext(ApplicationScoped.class, destroyObject);
> +
> +        //Destroy singleton context
>         endContext(Singleton.class, destroyObject);
> +
> +        //Clear saved contexts related with
> +        //this servlet context
> +        currentApplicationContexts.clear();
> +        currentSingletonContexts.clear();
>
> +        //Thread local values to null
>         requestContext.set(null);
> -        dependentContext.set(null);
>         sessionContext.set(null);
>         conversationContext.set(null);
>         applicationContext.set(null);
>         singletonContext.set(null);
>
> +        //Remove thread locals
> +        //for preventing memory leaks
>         requestContext.remove();
> -        dependentContext.remove();
>         sessionContext.remove();
>         conversationContext.remove();
>         applicationContext.remove();
>         singletonContext.remove();
>
> +        //remove dependent context
> +        dependentContext = null;
>     }
>
>
> @@ -165,6 +177,7 @@ public class WebContextsService extends
>             else if(scopeType.equals(Dependent.class))
>             {
>                 //Do nothing
> +                dependentContext = null;
>             }
>             else
>             {
> @@ -200,7 +213,7 @@ public class WebContextsService extends
>             }
>             else if(scopeType.equals(Dependent.class))
>             {
> -                return getDependentContext();
> +                return dependentContext;
>             }
>             else
>             {
> @@ -311,9 +324,8 @@ public class WebContextsService extends
>             {
>                 context.destroy();
>             }
> -
> -            requestContext.set(null);
>
> +            requestContext.set(null);
>         }
>     }
>
> @@ -396,25 +408,35 @@ public class WebContextsService extends
>      */
>     private void destroyApplicationContext(ServletContext servletContext)
>     {
> -        if (applicationContext != null)
> +        //look for thread local
> +        //this can be set by initRequestContext
> +        ApplicationContext context = applicationContext.get();
> +
> +        //no context set
> +        //look for saved application context
> +        if(context == null)
>         {
> -            ApplicationContext context = getApplicationContext();
> -
> -            if (context != null)
> +            if(servletContext != null)
>             {
> -                context.destroy();
> +                context = currentApplicationContexts.get(servletContext);
>             }
> -
> -            applicationContext.set(null);
> -
>         }
>
> +        //Destroy context
> +        if(context != null)
> +        {
> +            context.destroy();
> +        }
> +
> +        //Remove from saved contexts
>         if(servletContext != null)
>         {
>             currentApplicationContexts.remove(servletContext);
>         }
>
> +        //destroy all sessions
>         sessionCtxManager.destroyAllSessions();
> +        //destroy all conversations
>         conversationManager.destroyAllConversations();
>     }
>
> @@ -440,8 +462,7 @@ public class WebContextsService extends
>
>             }
>
> -            singletonContext.set(context);
> -
> +            singletonContext.set(context);
>         }
>
>     }
> @@ -452,19 +473,26 @@ public class WebContextsService extends
>      */
>     private void destroySingletonContext(ServletContext servletContext)
>     {
> -        if (singletonContext != null)
> +        SingletonContext context = getSingletonContext();
> +
> +        //no context set by initRequestContext
> +        if(context == null)
>         {
> -            SingletonContext context = getSingletonContext();
> -
> -            if (context != null)
> +            //look for saved context
> +            if(servletContext != null)
>             {
> -                context.destroy();
> +                context = currentSingletonContexts.get(servletContext);
>             }
> -
> -            singletonContext.set(null);
> -
>         }
>
> +        //context is not null
> +        //destroy it
> +        if(context != null)
> +        {
> +            context.destroy();
> +        }
> +
> +        //remove it from saved contexts
>         if(servletContext != null)
>         {
>             currentSingletonContexts.remove(servletContext);
> @@ -561,22 +589,4 @@ public class WebContextsService extends
>     {
>         return conversationContext.get();
>     }
> -
> -    /**
> -     * Gets dependent context.
> -     * @return dependent context
> -     */
> -    private DependentContext getDependentContext()
> -    {
> -        DependentContext dependentCtx = dependentContext.get();
> -
> -        if (dependentCtx == null)
> -        {
> -            dependentCtx = new DependentContext();
> -            dependentContext.set(dependentCtx);
> -        }
> -
> -        return dependentCtx;
> -    }
> -
>  }
> \ No newline at end of file
>
>
>