shiro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andreas Kohn (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (SHIRO-515) ExecutorServiceSessionValidationScheduler leaks resources due to improper synchronization
Date Fri, 18 Mar 2016 17:11:33 GMT

    [ https://issues.apache.org/jira/browse/SHIRO-515?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15201800#comment-15201800
] 

Andreas Kohn commented on SHIRO-515:
------------------------------------

SHIRO-443 looks like it describes/solves the same problem. That issue doesn't have any comments
from any Shiro developer either as of now :(

> 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.3.4#6332)

Mime
View raw message