[
https://issues.apache.org/jira/browse/ARTEMIS-4197?focusedWorklogId=992113&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-992113
]
ASF GitHub Bot logged work on ARTEMIS-4197:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 17/Nov/25 22:48
Start Date: 17/Nov/25 22:48
Worklog Time Spent: 10m
Work Description: jbertram opened a new pull request, #6080:
URL: https://github.com/apache/activemq-artemis/pull/6080
This commit mitigates a number of different potential race conditions
involving configuration changes. Consider the storeAddressSetting method in
o.a.a.a.c.p.i.j.AbstractJournalStorageManager. It mutates
mapPersistedAddressSettings twice - once to delete an entry and once to put an
entry. If another thread executes the recoverAddressSettings method in between
these two operations it will not contain the record that is being stored. This
same race condition exists for a number of different maps.
This commit mitigates the race condition by ensuring that the relevant map
is mutated only once (i.e. via put).
This commit also refactors a number of methods to avoid duplicated code.
There are no tests associated with this change since there's no
deterministic way to induce the race condition. It relies on static analysis
and existing tests to validate the fix and detect regressions.
Issue Time Tracking
-------------------
Worklog Id: (was: 992113)
Remaining Estimate: 0h
Time Spent: 10m
> AbstractJournalStorageManager address-settings not thread-safe
> --------------------------------------------------------------
>
> Key: ARTEMIS-4197
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4197
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Reporter: Jelmer Marinus
> Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> The AbstractJournalStorageManager contains the following code.
> {code:java}
> @Override
> public void storeAddressSetting(PersistedAddressSetting addressSetting)
> throws Exception {
> deleteAddressSetting(addressSetting.getAddressMatch());
> try (ArtemisCloseable lock = closeableReadLock()) {
> long id = idGenerator.generateID();
> addressSetting.setStoreId(id);
> bindingsJournal.appendAddRecord(id,
> JournalRecordIds.ADDRESS_SETTING_RECORD, addressSetting, true);
> mapPersistedAddressSettings.put(addressSetting.getAddressMatch(),
> addressSetting);
> }
> }{code}
> This code is used to update the AddressSettings through, i.e., the Artemis
> management API.
> The AbstractJournalStorageManager also contains the following code
> {code:java}
> @Override
> public List<PersistedAddressSetting> recoverAddressSettings() throws
> Exception {
> try (ArtemisCloseable lock = closeableReadLock()) {
> return new ArrayList<>(mapPersistedAddressSettings.values());
> }
> } {code}
> This code is called during, i.e., a config. reload when i.e. the broker.xml
> gets changed.
> When a first thread executes the first code fragement and a second thread
> executes the second fragment there is a possibility of the following:
> # The first thread executes the deleteAddressSettings for address MY_ADDRESS;
> # The second thread runs the whole recoverAddressSettings method
> # The first thread runs the remainder of the storeAddressSetting-method for
> MY_ADDRESS.
> According to the second thread there where no addressSettings for the address
> MY_ADDRESS. Because it ran in between of the delete and the put of the
> storeAddressSetting-method.
> I think this can be fixed using read-/write-locks from the storageManagerLock.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
