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

["ASF subversion and git services (Jira)" <ji...@apache.org>]

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

ASF subversion and git services commented on AMQ-9202:
------------------------------------------------------

Commit ed924cddac90b96bdc47b215852a68155d818bcd in activemq's branch refs/heads/main from Christopher L. Shannon (cshannon)
[ https://gitbox.apache.org/repos/asf?p=activemq.git;h=ed924cdda ]

AMQ-9202 - Make sure Reentrant locks are acquired outside a try block

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()


> 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: 20m
>  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)