[
https://issues.apache.org/jira/browse/ARTEMIS-4197?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18045556#comment-18045556
]
ASF subversion and git services commented on ARTEMIS-4197:
----------------------------------------------------------
Commit fd91f9fac558037ed48f0ca8e70dab62a60874eb in artemis's branch
refs/heads/main from Justin Bertram
[ https://gitbox.apache.org/repos/asf?p=artemis.git;h=fd91f9fac5 ]
ARTEMIS-4197 mitigate races w/various config changes
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.
> AbstractJournalStorageManager address-settings not thread-safe
> --------------------------------------------------------------
>
> Key: ARTEMIS-4197
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4197
> Project: Artemis
> Issue Type: Bug
> Reporter: Jelmer Marinus
> Assignee: Justin Bertram
> Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 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]
