Skip to content

[#2400] Make networkBridgeListener volatile for safe cross-thread publication - #2401

Open
mattrpav wants to merge 1 commit into
apache:mainfrom
mattrpav:amq-gh-2400-networkbridgelistener-volatile
Open

[#2400] Make networkBridgeListener volatile for safe cross-thread publication#2401
mattrpav wants to merge 1 commit into
apache:mainfrom
mattrpav:amq-gh-2400-networkbridgelistener-volatile

Conversation

@mattrpav

@mattrpav mattrpav commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

The listener is set from the connector setup thread and read from transport and executor threads via local snapshots without volatile those reads may miss.

…ad publication

The listener is set from the connector setup thread and read from
transport and executor threads via local snapshots; without volatile the
JMM does not guarantee those reads observe the write.
@cshannon

cshannon commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Does anything actually change the listener after bridge creation? I know there is a setter but I did a quick search and I didn't see anything using it after construction.

If there's nothing that will change the listener later then there is no need to make it volatile because during construction it's set and then added to the map under lock before start is called. If it's already set at that point then the lock creates a happens before (for anything using the same lock). Also when it's used by other threads, i think we use an executor and passing to an executor should also establish a happens-before.

So TLDR is unless something changes it after start is called (ie dynamically switching it out) I don't think it needs to be volatile. This might be a case where the listener should really just be part of the constructor if it's immutable anyways.

@mattrpav

mattrpav commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Bridge start is async in a thread, a fireBridgeFailed checks for null, but no log if it doesn't have it visible.

NetworkBridgeListener l = this.networkBridgeListener;
if (l != null && this.bridgeFailed.compareAndSet(false, true)) {
    l.bridgeFailed();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants