EpidemicServer uses inconsistent locks around listeners

[dclements]

registerListener synchronize around the this object, while the Subscription.cancel call does not need a lock. Meanwhile insertRevision has no synchronization, nor does its caller deliver, but its caller accept synchronizes around the lock object. The same basic pattern is true around acceptRevision and merge.

[dicej]

The registerListener/Subscription.cancel issue is definitely a bug, but I'm not seeing where the races are in the other cases you mentioned. Can you provide an example?

[dclements]

Inconsistent locking is pretty much always a bad practice (per JCIP). Listener registrations can occur while a revision is being inserted.

[dicej]

I see now. I hadn't noticed that both registerListener and the insertRevision operate on the same collection.