[PR] [fix][functions] Make exclusiveLeaderProducer volatile in FunctionMet… [pulsar]

["SongOf (via GitHub)" <gi...@apache.org>]

SongOf opened a new pull request, #26046:
URL: https://github.com/apache/pulsar/pull/26046

   PR Description
   
     ### Motivation
     
     In `FunctionMetaDataManager`, the `exclusiveLeaderProducer` field serves a
     double duty: a non-null value means this worker is the leader, a null value
     means it is not. It is the sole signal the write path uses to decide whether
     this worker may publish to the function metadata topic.
   
     The field is written by `acquireLeadership()`, which is *intentionally* not
     `synchronized`: it waits for the metadata tailer thread to drain
     (`tailer.stopWhenNoMoreMessages().get()`), and holding the monitor there would
     deadlock with the tailer thread, which calls the `synchronized`
     `processUpdate()` / `processDeregister()`. The field is then read by the
     `synchronized` methods `updateFunctionOnLeader()`, `start()` and
     `giveupLeadership()`.
   
     Because the write in `acquireLeadership()` does not hold the monitor, there is
     no happens-before edge between that write and the synchronized reads. Under the
     Java Memory Model this permits a reader to observe a stale `null` after
     leadership has actually been acquired, and `updateFunctionOnLeader()` would then
     spuriously throw `IllegalStateException("Not the leader")`. The field is plain
     (non-volatile), unlike the sibling cross-thread field `lastMessageSeen`, which
     is already declared `volatile`.
   
     ### Modifications
   
     - Declared `FunctionMetaDataManager.exclusiveLeaderProducer` as `volatile`.
     
     This establishes the missing happens-before edge between the unsynchronized
     write in `acquireLeadership()` and the synchronized reads, so the leadership
     signal is always published safely. The change is limited to the field modifier;
     no logic, locking, public API, or threading model is changed, and it is
     consistent with how `lastMessageSeen` is already handled in the same class.
   
     ### Verifying this change
     
     - [x] Make sure that the change passes the CI checks.
   
     This change is a trivial correctness/hardening fix without dedicated test
     coverage. The defect is a Java Memory Model visibility hazard that cannot be
     reliably reproduced by a unit test: the reads happen inside `synchronized`
     blocks (acquire fence + field reload) and the JVM runs on cache-coherent
     hardware, so a failing reproduction cannot be constructed deterministically or
     intermittently. The fix is verified by inspection against the JMM and is
     covered by the existing `FunctionMetaDataManagerTest` for behavioral regression.
   
     ### Does this pull request potentially affect one of the following parts:
   
     - [ ] Dependencies (add or upgrade a dependency)
     - [ ] The public API
     - [ ] The schema
     - [ ] The default values of configurations
     - [ ] The threading model
     - [ ] The binary protocol
     - [ ] The REST endpoints
     - [ ] The admin CLI options
     - [ ] The metrics
     - [ ] Anything that affects deployment


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][fn] Make exclusiveLeaderProducer volatile in FunctionMetaDataManager [pulsar]

["lhotari (via GitHub)" <gi...@apache.org>]

lhotari merged PR #26046:
URL: https://github.com/apache/pulsar/pull/26046


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org