You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stratos.apache.org by Reka Thirunavukkarasu <re...@wso2.com> on 2015/06/24 11:06:59 UTC

Thread Synchronization issue when using Hierarchical Locking

Hi

Thought of sharing some details about $subject. So that it might help us to
identify or debug any issues in future. We faced a thread synchronization
issue when using Hierarchical locking. The code segment below caused the
issue:

public TopologyLock getTopologyLockForService(String serviceName, boolean
forceCreationIfNotFound) {
        TopologyLock topologyLock =
serviceNameToTopologyLockMap.get(serviceName);
        if (topologyLock == null && forceCreationIfNotFound) {
           * synchronized (TopologyLockHierarchy.class) {*
                if (topologyLock == null) {
                    topologyLock = new TopologyLock();
                    serviceNameToTopologyLockMap.put(serviceName,
topologyLock);
                }
            }
        }
        return topologyLock;
    }

If two threads try to initially access this getTopologyLockForService at
the same time  just after the server startup when try to acquire write
lock, there would be higher chance that they see the topologyLock as null
which is a thread local variable as this method is not thread safe. After
that one thread would enter into synchronized block and add the lock to
hierarchy and exit. But when the second one executes the synchronized
block, since it already has the lock as null due to the get operation is
executed before the synchronized block, it will try to create a lock object
again and add it to the hierarchy. Due to this lock object got overwritten
in the map by second thread, when thread one tries to remove the lock from
the newly created lock object where first thread doesn't hold any lock,
thread one will fail to remove the lock [1]. Then our ReadWriteLockMonitor
complains that lock didn't get removed even after 30s [2].

Solution:

Make the method *syncronized* as below:

public *synchronized* TopologyLock getTopologyLockForService(String
serviceName, boolean forceCreationIfNotFound) {
        TopologyLock topologyLock =
serviceNameToTopologyLockMap.get(serviceName);
        if (topologyLock == null && forceCreationIfNotFound) {
            topologyLock = new TopologyLock();
            serviceNameToTopologyLockMap.put(serviceName, topologyLock);

        }
        return topologyLock;
    }

This solves the issue that was faced earlier.

Please share your concerns, if the fix has any other impact.

[1] [2015-06-23 17:01:34,524]  WARN
{org.apache.stratos.common.concurrent.locks.ReadWriteLock} -  System
warning! Trying to release a lock which has not been taken by the same
thread: [lock-name] application [thread-id] 131 [thread-name]
pool-28-thread-6

[2] 2015-06-23 17:02:04,526] ERROR
{org.apache.stratos.common.concurrent.locks.ReadWriteLockMonitor} -  System
error, lock has not released for 30 seconds: [lock-name] application
[lock-type] Write [thread-id] 131 [thread-name] pool-28-thread-6
[stack-trace]
java.lang.Thread.getStackTrace(Thread.java:1589)
org.apache.stratos.common.concurrent.locks.ReadWriteLock.acquireWriteLock(ReadWriteLock.java:123)
org.apache.stratos.messaging.message.processor.application.updater.ApplicationsUpdater.acquireWriteLockForApplication(ApplicationsUpdater.java:93)
org.apache.stratos.messaging.message.processor.application.GroupInstanceCreatedProcessor.process(GroupInstanceCreatedProcessor.java:57)
org.apache.stratos.messaging.message.processor.MessageProcessorChain.process(MessageProcessorChain.java:61)
org.apache.stratos.messaging.message.receiver.application.ApplicationsEventMessageDelegator.run(ApplicationsEventMessageDelegator.java:70)
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
java.lang.Thread.run(Thread.java:745)

org.apache.stratos.common.exception.LockNotReleasedException
    at
org.apache.stratos.common.concurrent.locks.ReadWriteLockMonitor.checkTimeout(ReadWriteLockMonitor.java:72)
    at
org.apache.stratos.common.concurrent.locks.ReadWriteLockMonitor.run(ReadWriteLockMonitor.java:55)
    at
java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
    at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:304)
    at
java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:178)
    at
java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
    at
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
    at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
    at java.lang.Thread.run(Thread.java:745)

Thanks,
Reka




-- 
Reka Thirunavukkarasu
Senior Software Engineer,
WSO2, Inc.:http://wso2.com,
Mobile: +94776442007

Re: Thread Synchronization issue when using Hierarchical Locking

Posted by Isuru Haththotuwa <is...@apache.org>.
Nice work cracking this down Reka!

Your fix looks fine for me.

On Wed, Jun 24, 2015 at 2:36 PM, Reka Thirunavukkarasu <re...@wso2.com>
wrote:

> Hi
>
> Thought of sharing some details about $subject. So that it might help us
> to identify or debug any issues in future. We faced a thread
> synchronization issue when using Hierarchical locking. The code segment
> below caused the issue:
>
> public TopologyLock getTopologyLockForService(String serviceName, boolean
> forceCreationIfNotFound) {
>         TopologyLock topologyLock =
> serviceNameToTopologyLockMap.get(serviceName);
>         if (topologyLock == null && forceCreationIfNotFound) {
>            * synchronized (TopologyLockHierarchy.class) {*
>                 if (topologyLock == null) {
>                     topologyLock = new TopologyLock();
>                     serviceNameToTopologyLockMap.put(serviceName,
> topologyLock);
>                 }
>             }
>         }
>         return topologyLock;
>     }
>
> If two threads try to initially access this getTopologyLockForService at
> the same time  just after the server startup when try to acquire write
> lock, there would be higher chance that they see the topologyLock as null
> which is a thread local variable as this method is not thread safe. After
> that one thread would enter into synchronized block and add the lock to
> hierarchy and exit. But when the second one executes the synchronized
> block, since it already has the lock as null due to the get operation is
> executed before the synchronized block, it will try to create a lock object
> again and add it to the hierarchy. Due to this lock object got overwritten
> in the map by second thread, when thread one tries to remove the lock from
> the newly created lock object where first thread doesn't hold any lock,
> thread one will fail to remove the lock [1]. Then our ReadWriteLockMonitor
> complains that lock didn't get removed even after 30s [2].
>
> Solution:
>
> Make the method *syncronized* as below:
>
> public *synchronized* TopologyLock getTopologyLockForService(String
> serviceName, boolean forceCreationIfNotFound) {
>         TopologyLock topologyLock =
> serviceNameToTopologyLockMap.get(serviceName);
>         if (topologyLock == null && forceCreationIfNotFound) {
>             topologyLock = new TopologyLock();
>             serviceNameToTopologyLockMap.put(serviceName, topologyLock);
>
>         }
>         return topologyLock;
>     }
>
> This solves the issue that was faced earlier.
>
> Please share your concerns, if the fix has any other impact.
>
> [1] [2015-06-23 17:01:34,524]  WARN
> {org.apache.stratos.common.concurrent.locks.ReadWriteLock} -  System
> warning! Trying to release a lock which has not been taken by the same
> thread: [lock-name] application [thread-id] 131 [thread-name]
> pool-28-thread-6
>
> [2] 2015-06-23 17:02:04,526] ERROR
> {org.apache.stratos.common.concurrent.locks.ReadWriteLockMonitor} -  System
> error, lock has not released for 30 seconds: [lock-name] application
> [lock-type] Write [thread-id] 131 [thread-name] pool-28-thread-6
> [stack-trace]
> java.lang.Thread.getStackTrace(Thread.java:1589)
>
> org.apache.stratos.common.concurrent.locks.ReadWriteLock.acquireWriteLock(ReadWriteLock.java:123)
>
> org.apache.stratos.messaging.message.processor.application.updater.ApplicationsUpdater.acquireWriteLockForApplication(ApplicationsUpdater.java:93)
>
> org.apache.stratos.messaging.message.processor.application.GroupInstanceCreatedProcessor.process(GroupInstanceCreatedProcessor.java:57)
>
> org.apache.stratos.messaging.message.processor.MessageProcessorChain.process(MessageProcessorChain.java:61)
>
> org.apache.stratos.messaging.message.receiver.application.ApplicationsEventMessageDelegator.run(ApplicationsEventMessageDelegator.java:70)
>
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
>
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
> java.lang.Thread.run(Thread.java:745)
>
> org.apache.stratos.common.exception.LockNotReleasedException
>     at
> org.apache.stratos.common.concurrent.locks.ReadWriteLockMonitor.checkTimeout(ReadWriteLockMonitor.java:72)
>     at
> org.apache.stratos.common.concurrent.locks.ReadWriteLockMonitor.run(ReadWriteLockMonitor.java:55)
>     at
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
>     at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:304)
>     at
> java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:178)
>     at
> java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
>     at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
>     at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
>     at java.lang.Thread.run(Thread.java:745)
>
> Thanks,
> Reka
>
>
>
>
> --
> Reka Thirunavukkarasu
> Senior Software Engineer,
> WSO2, Inc.:http://wso2.com,
> Mobile: +94776442007
>
> --
> <%2B94776442007>
> <%2B94776442007>
> Thanks and Regards,
>
> Isuru H.
> <%2B94776442007>
> +94 716 358 048 <%2B94776442007>* <http://wso2.com/>*
>
>
> * <http://wso2.com/>*
>
>
> * <http://wso2.com/>*
>
>
>

Re: Thread Synchronization issue when using Hierarchical Locking

Posted by Imesh Gunaratne <im...@apache.org>.
A great finding Reka! Your fix looks good.

IMO we will need to schedule a Jenkins job to enable lock monitoring
feature and run an integration test in the nightly build to identify such
problems.

Thanks

On Wed, Jun 24, 2015 at 2:36 PM, Reka Thirunavukkarasu <re...@wso2.com>
wrote:

> Hi
>
> Thought of sharing some details about $subject. So that it might help us
> to identify or debug any issues in future. We faced a thread
> synchronization issue when using Hierarchical locking. The code segment
> below caused the issue:
>
> public TopologyLock getTopologyLockForService(String serviceName, boolean
> forceCreationIfNotFound) {
>         TopologyLock topologyLock =
> serviceNameToTopologyLockMap.get(serviceName);
>         if (topologyLock == null && forceCreationIfNotFound) {
>            * synchronized (TopologyLockHierarchy.class) {*
>                 if (topologyLock == null) {
>                     topologyLock = new TopologyLock();
>                     serviceNameToTopologyLockMap.put(serviceName,
> topologyLock);
>                 }
>             }
>         }
>         return topologyLock;
>     }
>
> If two threads try to initially access this getTopologyLockForService at
> the same time  just after the server startup when try to acquire write
> lock, there would be higher chance that they see the topologyLock as null
> which is a thread local variable as this method is not thread safe. After
> that one thread would enter into synchronized block and add the lock to
> hierarchy and exit. But when the second one executes the synchronized
> block, since it already has the lock as null due to the get operation is
> executed before the synchronized block, it will try to create a lock object
> again and add it to the hierarchy. Due to this lock object got overwritten
> in the map by second thread, when thread one tries to remove the lock from
> the newly created lock object where first thread doesn't hold any lock,
> thread one will fail to remove the lock [1]. Then our ReadWriteLockMonitor
> complains that lock didn't get removed even after 30s [2].
>
> Solution:
>
> Make the method *syncronized* as below:
>
> public *synchronized* TopologyLock getTopologyLockForService(String
> serviceName, boolean forceCreationIfNotFound) {
>         TopologyLock topologyLock =
> serviceNameToTopologyLockMap.get(serviceName);
>         if (topologyLock == null && forceCreationIfNotFound) {
>             topologyLock = new TopologyLock();
>             serviceNameToTopologyLockMap.put(serviceName, topologyLock);
>
>         }
>         return topologyLock;
>     }
>
> This solves the issue that was faced earlier.
>
> Please share your concerns, if the fix has any other impact.
>
> [1] [2015-06-23 17:01:34,524]  WARN
> {org.apache.stratos.common.concurrent.locks.ReadWriteLock} -  System
> warning! Trying to release a lock which has not been taken by the same
> thread: [lock-name] application [thread-id] 131 [thread-name]
> pool-28-thread-6
>
> [2] 2015-06-23 17:02:04,526] ERROR
> {org.apache.stratos.common.concurrent.locks.ReadWriteLockMonitor} -  System
> error, lock has not released for 30 seconds: [lock-name] application
> [lock-type] Write [thread-id] 131 [thread-name] pool-28-thread-6
> [stack-trace]
> java.lang.Thread.getStackTrace(Thread.java:1589)
>
> org.apache.stratos.common.concurrent.locks.ReadWriteLock.acquireWriteLock(ReadWriteLock.java:123)
>
> org.apache.stratos.messaging.message.processor.application.updater.ApplicationsUpdater.acquireWriteLockForApplication(ApplicationsUpdater.java:93)
>
> org.apache.stratos.messaging.message.processor.application.GroupInstanceCreatedProcessor.process(GroupInstanceCreatedProcessor.java:57)
>
> org.apache.stratos.messaging.message.processor.MessageProcessorChain.process(MessageProcessorChain.java:61)
>
> org.apache.stratos.messaging.message.receiver.application.ApplicationsEventMessageDelegator.run(ApplicationsEventMessageDelegator.java:70)
>
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
>
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
> java.lang.Thread.run(Thread.java:745)
>
> org.apache.stratos.common.exception.LockNotReleasedException
>     at
> org.apache.stratos.common.concurrent.locks.ReadWriteLockMonitor.checkTimeout(ReadWriteLockMonitor.java:72)
>     at
> org.apache.stratos.common.concurrent.locks.ReadWriteLockMonitor.run(ReadWriteLockMonitor.java:55)
>     at
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
>     at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:304)
>     at
> java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:178)
>     at
> java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
>     at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
>     at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
>     at java.lang.Thread.run(Thread.java:745)
>
> Thanks,
> Reka
>
>
>
>
> --
> Reka Thirunavukkarasu
> Senior Software Engineer,
> WSO2, Inc.:http://wso2.com,
> Mobile: +94776442007
>
>
>


-- 
Imesh Gunaratne

Senior Technical Lead, WSO2
Committer & PMC Member, Apache Stratos