Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cyclic dependency between SSO classes causes deadlock #4281

Closed
sgflt opened this issue Oct 22, 2019 · 3 comments
Closed

Cyclic dependency between SSO classes causes deadlock #4281

sgflt opened this issue Oct 22, 2019 · 3 comments
Milestone

Comments

@sgflt
Copy link
Contributor

sgflt commented Oct 22, 2019

Description


Issue may relate to #2881 and #4280

During debugging why SSO never expires I have found a path, that can cause denial of service.

Expected Outcome

No DoS

Current Outcome

"ContainerBackgroundProcessor[StandardEngine[glassfish-web].StandardHost[server].StandardContext[]]" #248 daemon prio=5 os_prio=0 tid=0x00007f4945cd6000 nid=0x18dd waiting for monitor entry [0x00007f497cf16000]
   java.lang.Thread.State: BLOCKED (on object monitor)
at org.apache.catalina.authenticator.SingleSignOn.lookup(SingleSignOn.java:616)
- waiting to lock <0x00000000f4d78408> (a java.util.HashMap)
at org.glassfish.web.ha.authenticator.HASingleSignOn.removeSession(HASingleSignOn.java:186)
at com.sun.enterprise.security.web.GlassFishSingleSignOn.sessionEvent(GlassFishSingleSignOn.java:333)
at org.apache.catalina.session.StandardSession.fireSessionEvent(StandardSession.java:2367)
at org.apache.catalina.session.StandardSession.expire(StandardSession.java:992)
- locked <0x00000000f7611558> (a org.glassfish.web.ha.session.management.FullHASession)
at org.apache.catalina.session.StandardSession.expire(StandardSession.java:862)
at org.apache.catalina.session.StandardSession.isValid(StandardSession.java:779)
at org.apache.catalina.session.StandardSession.getAttributeNames(StandardSession.java:1392)
at org.glassfish.web.ha.session.management.BaseHASession.superToString(BaseHASession.java:239)
at org.glassfish.web.ha.session.management.BaseHASession.toString(BaseHASession.java:256)
at org.apache.catalina.authenticator.SingleSignOnEntry.removeSession(SingleSignOnEntry.java:124)
- locked <0x00000000f76113e0> (a org.glassfish.web.ha.authenticator.HASingleSignOnEntry)
at org.glassfish.web.ha.authenticator.HASingleSignOnEntry.removeSession(HASingleSignOnEntry.java:140)
- locked <0x00000000f76113e0> (a org.glassfish.web.ha.authenticator.HASingleSignOnEntry)
at org.glassfish.web.ha.authenticator.HASingleSignOn.removeSession(HASingleSignOn.java:191)
at com.sun.enterprise.security.web.GlassFishSingleSignOn.sessionEvent(GlassFishSingleSignOn.java:333)
at org.apache.catalina.session.StandardSession.fireSessionEvent(StandardSession.java:2367)
at org.apache.catalina.session.StandardSession.expire(StandardSession.java:992)

"SingleSignOnExpiration" #166 daemon prio=5 os_prio=0 tid=0x0000000001868000 nid=0x188b waiting for monitor entry [0x00007f494ef75000]
   java.lang.Thread.State: BLOCKED (on object monitor)
at org.apache.catalina.authenticator.SingleSignOnEntry.isEmpty(SingleSignOnEntry.java:136)
- waiting to lock <0x00000000f76113e0> (a org.glassfish.web.ha.authenticator.HASingleSignOnEntry)
at com.sun.enterprise.security.web.GlassFishSingleSignOn.processExpires(GlassFishSingleSignOn.java:604)
- locked <0x00000000f4d78408> (a java.util.HashMap)
at com.sun.enterprise.security.web.GlassFishSingleSignOn.run(GlassFishSingleSignOn.java:697)
at java.lang.Thread.run(Thread.java:748) 

Steps to reproduce (Only for bug reports)

Samples

Context (Optional)

SingleSignOnEntry:121 calls toString on removed session
log.warning("session " + session.getId() + "found (and removed): " + removed);

BaseHASession:239 calls getAttributeNames() that is forwarded to StandardSession:1349 with isValid call.

There I think isValid has side effect because it is not simple getter but it's meaning is near "IsValidAndExpireIfNot".

So we call toString on session that is expired and clean up thread processes expired sessions. Clean up thread holds a lock on session cache and expired session tries to print itself in another thread but cannot because needs lock on cache.

Environment

  • Payara Version: 4.1.1.171+
@smillidge
Copy link
Contributor

That is a very old version of Payara. Is the same problem present on the latest Payara 5.x?

@fturizo
Copy link
Contributor

fturizo commented Feb 6, 2020

Considering that the requester fell unresponsive and that this issue was reported on a considerably older release of Payara Server, we'll close this issue. Please, verify if the problem is present in the current release and if this is the case raise a new issue with a detailed reproducer that can help us identify the problem.

@fturizo fturizo closed this as completed Feb 6, 2020
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
- implementation reflects javadoc now
- fixes deadlock caused by side effect of getter
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
- see javadoc in Session interface
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
- improves encapsulation of parent class
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
- fixes deadlock between SingleSignOnEntry:121 and BaseHASession:239
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
- isValid has side effect described in Session interface
- we cannot break contract
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
- partially fixes cyclic dependency between SingleSignOnEntry:121 and BaseHASession:239
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
- isValid has side effect described in Session interface
- we cannot break contract
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
sgflt added a commit to sgflt/Payara that referenced this issue Feb 9, 2020
- synchronization object should be final, that is what Sonar said
Pandrex247 pushed a commit to Pandrex247/Payara that referenced this issue May 18, 2020
- see javadoc in Session interface
Pandrex247 pushed a commit to Pandrex247/Payara that referenced this issue May 18, 2020
Pandrex247 pushed a commit to Pandrex247/Payara that referenced this issue May 18, 2020
- improves encapsulation of parent class
Pandrex247 pushed a commit to Pandrex247/Payara that referenced this issue May 18, 2020
- partially fixes cyclic dependency between SingleSignOnEntry:121 and BaseHASession:239
Pandrex247 pushed a commit to Pandrex247/Payara that referenced this issue May 18, 2020
Pandrex247 pushed a commit to Pandrex247/Payara that referenced this issue May 18, 2020
- isValid has side effect described in Session interface
- we cannot break contract
Pandrex247 pushed a commit to Pandrex247/Payara that referenced this issue May 18, 2020
Pandrex247 pushed a commit to Pandrex247/Payara that referenced this issue May 18, 2020
Pandrex247 pushed a commit to Pandrex247/Payara that referenced this issue May 18, 2020
Pandrex247 pushed a commit to Pandrex247/Payara that referenced this issue May 18, 2020
@rdebusscher
Copy link

Part of release 5.2020.2

@OndroMih OndroMih added this to the 5.2020.2 milestone Jul 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants