jackrabbit-oak-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Dürig <mdue...@apache.org>
Subject Re: svn commit: r1492987 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ oak-core/src/main/java/org/apache/jackrabbit/oak/spi/whiteboard/ oak-jcr/src/main/j...
Date Fri, 14 Jun 2013 09:41:28 GMT



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, jukka@apache.org 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();
>>           }
>>       }

Mime
View raw message