[jira] [Work logged] (AMQ-9202) Reentrant locks should always be locked outside of a try block

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/AMQ-9202?focusedWorklogId=842946&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-842946 ]

ASF GitHub Bot logged work on AMQ-9202:
---------------------------------------

                Author: ASF GitHub Bot
            Created on: 01/Feb/23 16:21
            Start Date: 01/Feb/23 16:21
    Worklog Time Spent: 10m 
      Work Description: cshannon opened a new pull request, #959:
URL: https://github.com/apache/activemq/pull/959

   This is best practice and will prevent unlock from being attempted inside of a finally block when the thread doesn't actually own the lock which can happen when the lock attempt throws an exception such as calling lockInterruptibly()




Issue Time Tracking
-------------------

            Worklog Id:     (was: 842946)
    Remaining Estimate: 0h
            Time Spent: 10m

> Reentrant locks should always be locked outside of a try block
> --------------------------------------------------------------
>
>                 Key: AMQ-9202
>                 URL: https://issues.apache.org/jira/browse/AMQ-9202
>             Project: ActiveMQ
>          Issue Type: Bug
>          Components: Broker
>    Affects Versions: 5.17.3
>            Reporter: Christopher L. Shannon
>            Assignee: Christopher L. Shannon
>            Priority: Major
>             Fix For: 5.18.0, 5.17.4
>
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> I saw an exception happen today that turned out to be improper use of Reentrant locks. Acquiring the lock should always be done before the try block so that the finally clause to unlock will never execute unless the lock is acquired. More info: https://stackoverflow.com/questions/31058681/java-locking-structure-best-pattern
> The issue is inside of AbstractRuntimeConfigurationBroker, for example: [https://github.com/apache/activemq/blob/d8ce1d9ff0fa2296f3c56b59602e5cddb6ffe4a9/activemq-runtime-config/src/main/java/org/apache/activemq/plugin/AbstractRuntimeConfigurationBroker.java#L81]
> I saw a IllegalMonitorStateException exception occur due to lockInterruptibly() throwing an exception so when the finally block tried to unlock it threw the IllegalMonitorStateException since it was not owned by the thread.
> I went through the rest of the code and most places are correct but a couple spots need to be fixed as well.



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