search

jurmous / etcd4j

Java / Netty client for etcd, the highly-available key value store for shared configuration and service discovery.
Apache License 2.0
267 stars 83 forks source link

fix the concurrent problem of handlers in ResponsePromise #188

Open mashirofang opened 5 years ago

mashirofang commented 5 years ago

187

just using a lock to control the concurrent operations

lburgazzoli commented 5 years ago

maybe we just need a CopyOnWriteList instead of the lock/unlock

mashirofang commented 5 years ago

ok , I would conside this better way tomorrow, and commit a new version

mashirofang commented 5 years ago

maybe we just need a CopyOnWriteList instead of the lock/unlock

I think only using CopyOnWriteList may not solve the problem. In this case we won't have any ConcurrentModificationException , but anothor problem might happen that the handler we want to add might be invoked twice both in event-group-thread and user-defined-thread (in a very low probability we addListener just after event-group-thread sets success/exception and before gets iterator of the list to loop ) . So methods such as "addListener" , "handlePromise" and "setException" must be invoked atomicity . Lock/unlock can solve the probleam , but may not good enough , I would spend more time to think it over.

mashirofang commented 5 years ago

@lburgazzoli Hello , I'd thought a lot ,but didn't finger out anyway better than lock/unlock . ReentrantLock is an cas operation , also used in CopyOnWriteList and I use it the same way to control concurrent cases .Anyhow I commit a new version that adjusts some code to reduse the area of the lock .