[ 
https://issues.apache.org/jira/browse/SLING-11620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Sagar Miglani updated SLING-11620:
----------------------------------
    Description: 
WebconsoleSecurityProvider's 
[ServiceListeners|https://github.com/apache/sling-org-apache-sling-extensions-webconsolesecurityprovider/blob/master/src/main/java/org/apache/sling/extensions/webconsolesecurityprovider/internal/ServicesListener.java]
 class has three fields of Listener (an inner class of {{ServiceListeners}} 
implementing {{{}ServiceListener{}}}) {{{}repositoryListener{}}}, 
{{{}authSupportListener{}}}, {{{}authListener{}}}.

Each of listener uses synchronized methods and on any {{ServiceChanged}} event 
it may call {{notifyChange}} method of {{ServiceListeners}} parent class which 
is also a synchronized method and in this method we are again calling all of 
the listener fields' {{getService}} sychronized methods

If two service events for these listeneres fields occur simulatenously a 
deadlock situation may arise.

eg:
Thread1:
 - repositoryListener -> serviceChanged
 - synchronized retainSerivce - (self lock acquired)
 - synchronized notifyChange - (parent's lock acquired)

Thread2:
 - authSupportListener -> serviceChanged
 - synchronized retainService - (self lock aquired)
 - synchronized notifyChange - (wait for thread 1 to release parent's lock)

Thread1:
 - synchronized (authSupportListener) getService - (wait for thread 2 to 
release the lock)

Tried to replicate the scenario [^WebconsoleServiceListeners.patch]. The test 
in this patch file fails due to timeout.
On taking thread snapshot you may see the thread blocked and waiting for each 
other. [^BlockedThreads.png]

Shouldn't these all listener fields and parent class be using the shared lock 
instead of sychronized methods?

  was:
WebconsoleSecurityProvider's 
[ServiceListeners|https://github.com/apache/sling-org-apache-sling-extensions-webconsolesecurityprovider/blob/master/src/main/java/org/apache/sling/extensions/webconsolesecurityprovider/internal/ServicesListener.java]
 class has three fields of Listener (an inner class of {{ServiceListeners}} 
implementing {{{}ServiceListener{}}}) {{{}repositoryListener{}}}, 
{{{}authSupportListener{}}}, {{{}authListener{}}}.

Each of listener uses synchronized methods and on any {{ServiceChanged}} event 
it may call {{notifyChange}} method of {{ServiceListeners}} parent class which 
is also a synchronized method and in this method we are again calling all of 
the listener fields' {{getService}} sychronized methods

If two service events for these listeneres fields occur simulatenously a 
deadlock situation may arise.

eg:
Thread1:
 - repositoryListener -> serviceChanged
 - synchronized retainSerivce - (self lock acquired)
 - synchronized notifyChange - (parent's lock acquired)
Thread2:
 - authSupportListener -> serviceChanged
 - synchronized retainService - (self lock aquired)
 - synchronized notifyChange - (wait for thread 1 to release parent's lock)
Thread1:
 - synchronized (authSupportListener) getService - (wait for thread 2 to 
release the lock)

Tried to replicate the scenario [^WebconsoleServiceListeners.patch]. The test 
in this patch file fails due to timeout.
On taking thread snapshot you may see the thread blocked and waiting for each 
other. [^BlockedThreads.png]

Shouldn't these all listener fields and parent class be using the shared lock 
instead of sychronized methods?


> Synchronization issue in Webconsole Security Provider
> -----------------------------------------------------
>
>                 Key: SLING-11620
>                 URL: https://issues.apache.org/jira/browse/SLING-11620
>             Project: Sling
>          Issue Type: Bug
>          Components: Extensions
>    Affects Versions: Web Console Security Provider 1.2.6
>            Reporter: Sagar Miglani
>            Priority: Major
>         Attachments: BlockedThreads.png, WebconsoleServiceListeners.patch
>
>
> WebconsoleSecurityProvider's 
> [ServiceListeners|https://github.com/apache/sling-org-apache-sling-extensions-webconsolesecurityprovider/blob/master/src/main/java/org/apache/sling/extensions/webconsolesecurityprovider/internal/ServicesListener.java]
>  class has three fields of Listener (an inner class of {{ServiceListeners}} 
> implementing {{{}ServiceListener{}}}) {{{}repositoryListener{}}}, 
> {{{}authSupportListener{}}}, {{{}authListener{}}}.
> Each of listener uses synchronized methods and on any {{ServiceChanged}} 
> event it may call {{notifyChange}} method of {{ServiceListeners}} parent 
> class which is also a synchronized method and in this method we are again 
> calling all of the listener fields' {{getService}} sychronized methods
> If two service events for these listeneres fields occur simulatenously a 
> deadlock situation may arise.
> eg:
> Thread1:
>  - repositoryListener -> serviceChanged
>  - synchronized retainSerivce - (self lock acquired)
>  - synchronized notifyChange - (parent's lock acquired)
> Thread2:
>  - authSupportListener -> serviceChanged
>  - synchronized retainService - (self lock aquired)
>  - synchronized notifyChange - (wait for thread 1 to release parent's lock)
> Thread1:
>  - synchronized (authSupportListener) getService - (wait for thread 2 to 
> release the lock)
> Tried to replicate the scenario [^WebconsoleServiceListeners.patch]. The test 
> in this patch file fails due to timeout.
> On taking thread snapshot you may see the thread blocked and waiting for each 
> other. [^BlockedThreads.png]
> Shouldn't these all listener fields and parent class be using the shared lock 
> instead of sychronized methods?



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to