shiro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kalle Korhonen <kalle.o.korho...@gmail.com>
Subject Re: Shiro web apps - static SecurityManager by default?
Date Fri, 06 May 2011 16:10:07 GMT
Agree with Jared. Please don't enable static by default - it can have
a lot of unforeseen consequences with reloading
classes/webapplications and can easily cause memory leaks.

Kalle


On Thu, May 5, 2011 at 3:21 PM, Jared Bunting
<jared.bunting@digitalreasoning.com> wrote:
> I know it's a bit more in depth than what you're asking, but what if,
> rather than making it a static variable by default, we made it available
> as a ServletContext attribute?
>
> Since the use case for this seems to be running arbitrary threads inside
> a webapp, and those are started either by a ServletContextListener or by
> a DI container that has access to the ServletContext, it would make it
> very easy to do something like this:
>
> public class MyServletContextListener implements ServletContextListener {
>
>  private ExecutorService executorService =
> Executors.newSingleThreadExecutor();
>
>  public void contextInitialized(ServletContextEvent event) {
>    Subject subject = new
> Subject.Builder(WebUtils.retrieveSecurityManager(event.getServletContext())).buildSubject();
>    Runnable myRunnable = subject.associateWith(new MyRunnable());
>    executorService.execute(myRunnable);
>  }
>
>  public void contextDestroyed(ServletContextEvent event) {
>    executorService.shutdownNow();
>    executorService.awaitTermination(10, TimeUnit.SECONDS);
>  }
> }
>
>
> Retrieving the security manager is slightly more complicated than a
> static variable, but in my opinion it's safer and confines your
> SecurityManager to the appropriate scope.
>
> -Jared
>
>
> On 05/05/2011 05:01 PM, Les Hazlewood wrote:
>> I just resolved SHIRO-287[1] which enables the SecurityManager set up
>> by the ShiroFilter to be statically accessible to the web app.  This
>> ensures that SecurityUtils.getSubject()/getSecurityManager() will very
>> easily work even on non-request threads.
>>
>> However, I left this disabled by default to retain the existing
>> behavior (no static memory).
>>
>> Jared raised the idea that this could probably be enabled by default
>> because the only time there would be static memory conflict is if
>> there is more than one Shiro web app in the same servlet container and
>> both of those webapps use a shared classloader - probably something
>> that occurs quite rarely, if at all, these days.
>>
>> That is, if static memory conflict lies in the 20% use case (probably
>> more like 5% or 1%), then in keeping with Shiro's 'it just works'
>> objective, it'd be easier for the other 80%/95% of people if it was
>> enabled by default.
>>
>> It is still disabled by default, but I was wondering if anyone wanted
>> to chime in here.  Thoughts?
>>
>> [1] https://issues.apache.org/jira/browse/SHIRO-287
>>
>> Cheers,
>>
>
>

Mime
View raw message