boris-petrov commented on pull request #218:
URL: https://github.com/apache/shiro/pull/218#issuecomment-621723281
@fpapon - thanks for the time.
Exactly the reference is the problem. Marking this field as volatile means
that whenever some thread sets the value (using `setSecurityManager`) all other
threads from that time on will "see" the correct security manager. Otherwise,
they can "see" a previous one.
And that's exactly what happens in our tests. We set initially a security
manager, then we destroy it and set another one for the rest of the time. A few
minutes later, a thread does `new
Subject.Builder().sessionCreationEnabled(false).sessionId(shiroSessionId).buildSubject()`
which blows up with a NPE:
```
java.lang.NullPointerException: null
at
org.apache.shiro.mgt.SessionsSecurityManager.getSession(SessionsSecurityManager.java:156)
at
org.apache.shiro.mgt.DefaultSecurityManager.resolveContextSession(DefaultSecurityManager.java:461)
at
org.apache.shiro.mgt.DefaultSecurityManager.resolveSession(DefaultSecurityManager.java:447)
at
org.apache.shiro.mgt.DefaultSecurityManager.createSubject(DefaultSecurityManager.java:343)
at
org.apache.shiro.subject.Subject$Builder.buildSubject(Subject.java:845)
at com.company.SomeClass.buildSubject(SomeClass.java:321)
...
```
Because it still "sees" the previously set (and destroyed) security manager
even though it's been a long time since a new one has been set.
Marking this field as volatile is practically free and is making the code
work correctly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
