Maybe wrong usage of ConcurrentHashMap in DomainServicesManager

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Maybe wrong usage of ConcurrentHashMap in DomainServicesManager

James Mackerel

https://github.com/apereo/cas/blob/168cae1194e027e9a6106114fca30104b1811d1b/core/cas-server-core-services-registry/src/main/java/org/apereo/cas/services/domain/DefaultDomainAwareServicesManager.java#L94

It seems that the only concurrent read/write operation is in the line shown above. But this operation can cause thread safe problem, for example:

- thread 1 clears domain map
- at this moment, OS suspends thread 1 an wakes up thread 2
- thread 2 calls getCandidateServicesToMatch, and got a empty domain list

The correct way may be making domains map a volatile reference (volatile may not be required), and use normal HashMap (no need to use ConcurrentHashMap because there is no concurrent read/write on this object), prepare new domains map and assign to  this.domains on updating.

Similarly, services List in AbstractServicesManager has same problem of wrong usage of ConcurrentHashMap, but there is no thread safety problem because services list is assigned rather than "clean and putAll" on updating.

Please let me known if I had any misunderstanding of this machanism, thank you.


--
You received this message because you are subscribed to the Google Groups "CAS Developer" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/a/apereo.org/d/msgid/cas-dev/0ba29010-a8f7-4a87-ad0f-0fffe1c53026n%40apereo.org.