[CURATOR-495] Fixes race in many Curator recipes which could block ZK's event thread#297
[CURATOR-495] Fixes race in many Curator recipes which could block ZK's event thread#297
Conversation
Fixes race in many Curator recipes whereby a pattern was used that called "notifyAll" in a synchronized block in response to a ZooKeeper watcher callback. This created a race and possible deadlock if the recipe instance was already in a synchronized block. This would result in the ZK event thread getting blocked which would prevent ZK connections from getting repaired. This change adds a new executor (available from CuratorFramework) that can be used to do the sync/notify so that ZK's event thread is not blocked.
| * @param monitorHolder object to sync on and notify | ||
| * @since 4.1.0 | ||
| */ | ||
| default void postSafeNotify(Object monitorHolder) |
There was a problem hiding this comment.
Thinking about the future...
Would it be better to make it return a CompletableFuture?
This way the caller will have a chance to know when eventually the operation has been executed
There was a problem hiding this comment.
Good idea - I'll add that
… can check completion, etc.
eolivelli
left a comment
There was a problem hiding this comment.
+1 (non binding)
Awesome!
curator-framework/src/main/java/org/apache/curator/framework/CuratorFrameworkFactory.java
Outdated
Show resolved
Hide resolved
|
I have just run the tests and everything passed, which is a bit of a novelty! Still failed at the end with that surefire issue. I think that this is good to merge though. Nice work! |
|
For the record: here’s the Surefire bug report: https://issues.apache.org/jira/browse/SUREFIRE-1612 |
|
Did you try to upgrade testng and surefire to latest and greatest version? |
Surefire was updated as part of getting the latest Apache POM. I'll play around with latest TestNG |
|
FYI - I just tried it with the latest TestNg and it still fails. So, I re-opened my issue there: testng-team/testng#1976 |
issue: apache#2020 curator issue: apache/curator#297
* upgrade curator version issue: #2020 curator issue: apache/curator#297 * add DefaultEnsembleProvider override * add some unit test * add License
Fixes race in many Curator recipes whereby a pattern was used that called "notifyAll" in a synchronized block in response to a ZooKeeper watcher callback. This created a race and possible deadlock if the recipe instance was already in a synchronized block. This would result in the ZK event thread getting blocked which would prevent ZK connections from getting repaired. This change adds a new executor (available from CuratorFramework) that can be used to do the sync/notify so that ZK's event thread is not blocked.