visit2rahul commented on code in PR #4498:
URL: https://github.com/apache/polaris/pull/4498#discussion_r3284648595
##########
runtime/service/src/main/java/org/apache/polaris/service/events/listeners/inmemory/InMemoryBufferEventListener.java:
##########
@@ -68,7 +75,17 @@ protected void processEvent(String realmId, PolarisEvent
event) {
@PreDestroy
public void shutdown() {
- processors.asMap().values().forEach(UnicastProcessor::onComplete);
+ // Same rationale as the eviction listener: hold the processor's monitor so
+ // shutdown-driven onComplete cannot race a concurrent processEvent-driven
onNext.
+ processors
+ .asMap()
+ .values()
+ .forEach(
+ p -> {
+ synchronized (p) {
Review Comment:
Thank you @flyrain. Good suggestion - on reflection the helper isn't about
line count but about encoding the synchronization as an invariant: any future
caller adding a `processor.onComplete()` would have to actively bypass the
helper rather than silently omit the `synchronized` wrapper, which is the right
safety property for a thread-correctness fix. Extracted to
`completeSynchronized` in 69fdd71c9.
Could you trigger the CI workflow run on this push when you have a moment?
--
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]
