AFAIU to comply with the removeEventListener contract we either have to interrupt the observation thread (like my implementation did) or live with the deadlock caused by client handlers blocking on events (like you experienced with RepositoryTest.observationDispose).
Michael On 14.6.13 10:23, Michael Dürig wrote:
Hi, Below code breaks stop()'s contract, which states that no further events will be delivered after calling stop. It will be that way probably most of the time but not necessarily always. Changing stop()'s contract escalates to ObservationManagerImpl.removeEventListener, which explicitly states "The deregistration method will block until the listener has completed executing." It also escalates to Session.logout leading to the (rare but confusing) possibility of events being still delivered to "logged out" listeners. Michael On 14.6.13 8:51, [email protected] wrote:@@ -108,29 +109,17 @@ class ChangeProcessor implements Runnabl * events will be delivered. * @throws IllegalStateException if not yet started or stopped already */ - public synchronized void stop() { - if (future == null) { - throw new IllegalStateException("Change processor not started"); - } - - try { - stopping = true; - future.cancel(true); - while (running) { - wait(); - } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - finally { + public void stop() { + stopping = true; // do this outside synchronization + synchronized (this) { + checkState(registration != null, "Change processor not started"); changeListener.dispose(); - future = null; + registration.unregister(); } }
