BewareMyPower commented on PR #22279: URL: https://github.com/apache/pulsar/pull/22279#issuecomment-2001758988
@lhotari `scheduleReadOnActiveConsumer` is thread safe. The steps of `scheduleReadOnActiveConsumer` are: 1. Call `cancelPendingRead()`, which might set `havePendingRead` with false. (The concurrent call of `cancelPendingRead()` does not matter because it does not set `havePendingRead` with true so that following logic will be skipped.) 2. Skip the following logic if `havePendingRead` is true, which could only be caused by `readMoreEntries`. 3. Call `readMoreEntries()` immediately or lazily according to the subscription type and the config. Yes, there is a chance that `readMoreEntries` is called concurrently. | Steps | Thread 1 | Thread 2 | | :- | :- | :- | | 1 | Read `havePendingRead`: false | | | 2 | | Write `havePendingRead`: true | | 3 | Call `readMoreEntries` | | However, it's still safe because in `readMoreEntries`, before reading messages through `cursor`, it checks `havePendingRead` again in the synchronized block. https://github.com/apache/pulsar/blob/442595ea26c8c0699807a0fef2b7e2e27c677c08/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherSingleActiveConsumer.java#L343-L349 > if the same bug pattern is in other locations in the Pulsar code base? BTW, yes, the same bug pattern is use in many places. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
