[#2400] Make networkBridgeListener volatile for safe cross-thread publication - #2401
[#2400] Make networkBridgeListener volatile for safe cross-thread publication#2401mattrpav wants to merge 1 commit into
Conversation
…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.
|
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. |
|
Bridge start is async in a thread, a fireBridgeFailed checks for null, but no log if it doesn't have it visible. |
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.