Nice work cracking this down Reka!
Your fix looks fine for me.
On Wed, Jun 24, 2015 at 2:36 PM, Reka Thirunavukkarasu <r...@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/>*
>
>
>
