[
https://issues.apache.org/jira/browse/SHIRO-515?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andreas Kohn updated SHIRO-515:
-------------------------------
Summary: ExecutorServiceSessionValidationScheduler leaks resources due to
improper synchronization (was: ExecutorServiceSessionValidationScheduler
instances leaking due to improper synchronization)
> ExecutorServiceSessionValidationScheduler leaks resources due to improper
> synchronization
> -----------------------------------------------------------------------------------------
>
> Key: SHIRO-515
> URL: https://issues.apache.org/jira/browse/SHIRO-515
> Project: Shiro
> Issue Type: Bug
> Components: Session Management
> Affects Versions: 1.2.3
> Reporter: Andreas Kohn
> Priority: Critical
> Labels: patch
> Attachments: SHIRO-515-race.patch
>
>
> Related to SHIRO-514, which talks about a bunch of threads with "ugly" names.
> This ticket is about _why_ there are multiple of those in the first place.
> From my heap dump I can see about 71 ThreadFactory instances of
> ExecutorServiceSessionValidationScheduler$1, each actually linking to a
> distinct ScheduledThreadPoolExecutor, and each running one thread.
> However, there are only 2 ExecutorServiceSessionValidationScheduler instances.
> The problem is in
> AbstractValidatingSessionManager#enableSessionValidationIfNecessary(): if
> this method is called from multiple threads, and those threads race in such a
> way that multiple ones see an unset/not-yet-enabled scheduler, they will each
> run #enableSessionValidation(), which creates a scheduler, sets it (still
> disabled), and then runs
> SessionValidationScheduler#enableSessionValidation(), which might set its
> 'enabled' flag to true. In the case of the default
> ExecutorServiceSessionValidationScheduler this creates the thread pool
> executor, and schedules the validation run iff there is a positive interval
> configured.
> The race is not particularly unlikely when an application using shiro is
> deployed and many requests hit it immediately. As Shiro is leaking both JVM
> and system resources here it's also not a benign race.
> The attached patch changes the sessionValidationScheduler to be volatile, and
> makes #enableSessionValidation() as well as #disableSessionValidation()
> {{synchronized}}.
> #enableSessionValidationIfNecessary() is not synchronized in the common case
> to avoid too much blocking when everything is properly warmed up, but rather
> uses double-checked locking. Shiro targets a 1.5 JVM from what I can see, so
> this should be ok.
> Note that callers can possibly still produce issues by modifying the field
> directly.
> Additionally the ExecutorServiceSessionValidationScheduler was changed to
> report 'enabled' when someone tried enabling the validation, regardless of
> whether the interval was 0 or not. The rationale here is that someone might
> try disabling the validation by setting the interval, which would mean
> useless calls to #enableSessionValidation() in every request.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
