ConcurrentModificationException, Cannot begin a new event ... - reproducible

[Emil Crumhorn]

I think I may have found a reproducible "test case" for the following exception:

java.util.ConcurrentModificationException: Cannot begin a new event while another event is in progress by thread, main

I have my glazed lists update/replace/delete methods wrapped in a class so that everything is always locked, and the method I use to reproduce the exceptions is this;

/**
     * Replaces one entry with another in the same position of the table. Does nothing if the old object is not found in
     * the table.
     * 
     * @param oldEntry Old object to replace
     * @param newEntryNew replacement object
     * @param index index of message to replace, -1 to have it calculated for you (slow)
     */
    public boolean replace(IDataObject oldEntry, IDataObject newEntry, int index) {
        try {
            _eventList.getReadWriteLock().writeLock().lock();
            if (index == -1) {
                index = _eventList.indexOf(oldEntry);
            }

            if (index != -1) {
                _eventList.set(index, newEntry);
                return true;
            }
        }
        catch (Exception err) {
            LogManager.error(err);
        }
        finally {
            _eventList.getReadWriteLock().writeLock().unlock();
        }

        return false;
    }

As you can see from the method signature, it either takes an index or tries to find it. In my test code I continuously (once per second) replace 20 items in a table with 20 new items, from position 20 to 1 (or 19 to 0 if you will) (we show newest entries at the top in the table).

As our tables can be really large, we keep a Primary Key index that's updated every time the list is modified so that they match, thus we can find the index faster than the table can via object comparison.

In any case, one tester reported the exception and I couldn't for the life of me reproduce it until I got his full log and saw that the originating exception was a replace call with an index that was larger than the event list itself;

java.lang.IndexOutOfBoundsException: Index: 35, Size: 26
	at java.util.ArrayList.RangeCheck(ArrayList.java:547)
	at java.util.ArrayList.set(ArrayList.java:337)
	at ca.odell.glazedlists.BasicEventList.set(BasicEventList.java:188)

And following that exception, tons of the ConcurrentModificationExceptions.

So what I did was that in our loop that calls replace() I halted the code in debug mode and forced an index to be 21 (table has 20 entries, so 21 is out of bounds) and then I traced it to see what happened.

First IndexOutOfBounds is thrown when it tries to do _eventList.set(index, newEntry); That's expected. Then it reaches _eventList.getReadWriteLock().writeLock().unlock(); and returns false. All expected.

However, on the next iteration to replace, even if the update index is valid (code paused, forced 21 index removed), the ConcurrentModificationException is thrown every time for every method call.

So I guess my question is twofold;

1. Is there something I'm doing obviously wrong in the above code?
2. Is something not reset with the event list when it gets an IndexOutOfBounds exception that's the cause of this? It seems to me if the list is unlocked again there shouldn't be a double modification on the next iteration.

It's worth adding that all of this is SWT RCP code with a Nat table behind it, so table updates are originating from a UI thread.

Regards,
Emil

[Holger Brands]

Emil,

>
> In any case, one tester reported the exception and I couldn't for the life
> of me reproduce it until I got his full log and saw that the originating
> exception was a replace call with an index that was larger than the event
> list itself;
>
>
> java.lang.IndexOutOfBoundsException: Index: 35, Size: 26
> at java.util.ArrayList.RangeCheck(ArrayList.java:547)
> at java.util.ArrayList.set(ArrayList.java:337)
> at ca.odell.glazedlists.BasicEventList.set(BasicEventList.java:188)
>
>
> And following that exception, tons of the ConcurrentModificationExceptions.
>

By passing a wrong index to BasicEventList.set(...) the Glazed Lists
event delivery subsystem is left in an inconsistent state because of the
IndexOutOfBoundsException.
So, to resolve your issue, you'd have to find the place,
where the wrong index originates from.
If you can fix that, those ConcurrentModificationExceptions will disappear, too.

Hope this helps,
Holger

__________________________________________________________________________
Verschicken Sie SMS direkt vom Postfach aus - in alle deutschen und viele
ausländische Netze zum gleichen Preis!
https://produkte.web.de/webde_sms/sms




---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@...
For additional commands, e-mail: users-help@...

[Emil Crumhorn]

Hi Holger,

Thanks, but I was hoping to clarify in my post that even if I start passing the correct index after having passed the wrong index just once, the list keeps throwing Concurrent exceptions for every set call on the list even if the index is valid for subsequent calls.

Is this the expected behavior? it seems one should be able to make a "mistake" without getting the list into a permanent state of exception throwing.

Emil

Holger Brands wrote:

Emil,

>
> In any case, one tester reported the exception and I couldn't for the life
> of me reproduce it until I got his full log and saw that the originating
> exception was a replace call with an index that was larger than the event
> list itself;
>
>
> java.lang.IndexOutOfBoundsException: Index: 35, Size: 26
> at java.util.ArrayList.RangeCheck(ArrayList.java:547)
> at java.util.ArrayList.set(ArrayList.java:337)
> at ca.odell.glazedlists.BasicEventList.set(BasicEventList.java:188)
>
>
> And following that exception, tons of the ConcurrentModificationExceptions.
>

By passing a wrong index to BasicEventList.set(...) the Glazed Lists
event delivery subsystem is left in an inconsistent state because of the
IndexOutOfBoundsException.
So, to resolve your issue, you'd have to find the place,
where the wrong index originates from.
If you can fix that, those ConcurrentModificationExceptions will disappear, too.

Hope this helps,
Holger

__________________________________________________________________________
Verschicken Sie SMS direkt vom Postfach aus - in alle deutschen und viele
ausländische Netze zum gleichen Preis!
https://produkte.web.de/webde_sms/sms




---------------------------------------------------------------------
To unsubscribe, e-mail: [e-mail]
For additional commands, e-mail: [e-mail]

[Holger Brands]

 > Is this the expected behavior?

Yes, with the current implementation, that's expected.
If you dive into the implementation you'll see this:

    public E set(int index, E element) {
        // create the change event
        updates.beginEvent();
        // do the actual set
        E previous = data.set(index, element);
        // fire the event
        updates.elementUpdated(index, previous);
        updates.commitEvent();
        return previous;
    }

Event delivery is guarded by calls to
ListEventAssembler.beginEvent() - start recording changes
and
ListEventAssembler.commitEvent() - propagate recorded changes

When an uncaught exception is thrown inbetween, commitEvent()
is never called and no change events are fired.
With the next call to beginEvent() the ListEventAssembler detects,
that an event delivery is in progress leading to the ConcurrentModificationExceptions
you observed.

Holger
__________________________________________________________________________
Verschicken Sie SMS direkt vom Postfach aus - in alle deutschen und viele
ausländische Netze zum gleichen Preis!
https://produkte.web.de/webde_sms/sms




---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@...
For additional commands, e-mail: users-help@...

[Endre Stølsvik-8]

I've been wondering about this: shouldn't the beginEvent-commitEvent
always be in a try-finally block? This to contain any errors as best
one can? It seems to me like any error anywhere in the pipeline now
will lead to an irreparably inconsistent state, while if one had
try-finally, with the updates.["change"] always after the actual data
change invocation method, one would get something that at least in
several situations resembles a transaction with both commit and
rollback semantics.

This would be so that even though there are some logic errors to could
come up in certain situations, the application using GL would still
have a chance of being consistent after an error - of being able to
recover out of errors.

Endre.

On Sun, Apr 26, 2009 at 11:06, Holger Brands <hbrands@...> wrote:

>  > Is this the expected behavior?
>
> Yes, with the current implementation, that's expected.
> If you dive into the implementation you'll see this:
>
>    public E set(int index, E element) {
>        // create the change event
>        updates.beginEvent();
>        // do the actual set
>        E previous = data.set(index, element);
>        // fire the event
>        updates.elementUpdated(index, previous);
>        updates.commitEvent();
>        return previous;
>    }
>
> Event delivery is guarded by calls to
> ListEventAssembler.beginEvent() - start recording changes
> and
> ListEventAssembler.commitEvent() - propagate recorded changes
>
> When an uncaught exception is thrown inbetween, commitEvent()
> is never called and no change events are fired.
> With the next call to beginEvent() the ListEventAssembler detects,
> that an event delivery is in progress leading to the ConcurrentModificationExceptions
> you observed.
>
> Holger
> __________________________________________________________________________
> Verschicken Sie SMS direkt vom Postfach aus - in alle deutschen und viele
> ausländische Netze zum gleichen Preis!
> https://produkte.web.de/webde_sms/sms
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@...
> For additional commands, e-mail: users-help@...
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@...
For additional commands, e-mail: users-help@...

[Emil Crumhorn]

I completely agree. It seems to me it should be possible to make a "mistake" without getting the entire list system out of order. Normal java lists just continue to work even if you mess up the index (after throwing an exception of course).

Unless there's a big reason why the commit can't finish despite the error, it seems to me this would be an easy fix and at least you'd get the lists back on track. Now I have to explicitly check indexes before a set just so that I never get an IndexOutOfBounds exception, as if I get one, then I can't use the lists anymore.

Emil

Endre Stølsvik-8 wrote:

I've been wondering about this: shouldn't the beginEvent-commitEvent
always be in a try-finally block? This to contain any errors as best
one can? It seems to me like any error anywhere in the pipeline now
will lead to an irreparably inconsistent state, while if one had
try-finally, with the updates.["change"] always after the actual data
change invocation method, one would get something that at least in
several situations resembles a transaction with both commit and
rollback semantics.

This would be so that even though there are some logic errors to could
come up in certain situations, the application using GL would still
have a chance of being consistent after an error - of being able to
recover out of errors.

Endre.

On Sun, Apr 26, 2009 at 11:06, Holger Brands <[e-mail]> wrote:
>  > Is this the expected behavior?
>
> Yes, with the current implementation, that's expected.
> If you dive into the implementation you'll see this:
>
>    public E set(int index, E element) {
>        // create the change event
>        updates.beginEvent();
>        // do the actual set
>        E previous = data.set(index, element);
>        // fire the event
>        updates.elementUpdated(index, previous);
>        updates.commitEvent();
>        return previous;
>    }
>
> Event delivery is guarded by calls to
> ListEventAssembler.beginEvent() - start recording changes
> and
> ListEventAssembler.commitEvent() - propagate recorded changes
>
> When an uncaught exception is thrown inbetween, commitEvent()
> is never called and no change events are fired.
> With the next call to beginEvent() the ListEventAssembler detects,
> that an event delivery is in progress leading to the ConcurrentModificationExceptions
> you observed.
>
> Holger
> __________________________________________________________________________
> Verschicken Sie SMS direkt vom Postfach aus - in alle deutschen und viele
> ausländische Netze zum gleichen Preis!
> https://produkte.web.de/webde_sms/sms
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [e-mail]
> For additional commands, e-mail: [e-mail]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [e-mail]
For additional commands, e-mail: [e-mail]

[Holger Brands]

I agree that a better error recovery is desirable.
I just wonder if it's that easy to fix.
If it were, the original authors would have done that already, I guess.

Perhaps James and/or Jesse can shed some light on this topic.
Maybe at least some list implementations like BasicEventList could
be enhanced...

Holger

>
> I completely agree. It seems to me it should be possible to make a "mistake"
> without getting the entire list system out of order. Normal java lists just
> continue to work even if you mess up the index (after throwing an exception
> of course).
>
> Unless there's a big reason why the commit can't finish despite the error,
> it seems to me this would be an easy fix and at least you'd get the lists
> back on track. Now I have to explicitly check indexes before a set just so
> that I never get an IndexOutOfBounds exception, as if I get one, then I
> can't use the lists anymore.
>
> Emil
>
>
> Endre Stølsvik-8 wrote:
> >
> > I've been wondering about this: shouldn't the beginEvent-commitEvent
> > always be in a try-finally block? This to contain any errors as best
> > one can? It seems to me like any error anywhere in the pipeline now
> > will lead to an irreparably inconsistent state, while if one had
> > try-finally, with the updates.["change"] always after the actual data
> > change invocation method, one would get something that at least in
> > several situations resembles a transaction with both commit and
> > rollback semantics.
> >
> > This would be so that even though there are some logic errors to could
> > come up in certain situations, the application using GL would still
> > have a chance of being consistent after an error - of being able to
> > recover out of errors.
> >
> > Endre.
> >
> > On Sun, Apr 26, 2009 at 11:06, Holger Brands <hbrands@...> wrote:
> >>  > Is this the expected behavior?
> >>
> >> Yes, with the current implementation, that's expected.
> >> If you dive into the implementation you'll see this:
> >>
> >>    public E set(int index, E element) {
> >>        // create the change event
> >>        updates.beginEvent();
> >>        // do the actual set
> >>        E previous = data.set(index, element);
> >>        // fire the event
> >>        updates.elementUpdated(index, previous);
> >>        updates.commitEvent();
> >>        return previous;
> >>    }
> >>
> >> Event delivery is guarded by calls to
> >> ListEventAssembler.beginEvent() - start recording changes
> >> and
> >> ListEventAssembler.commitEvent() - propagate recorded changes
> >>
> >> When an uncaught exception is thrown inbetween, commitEvent()
> >> is never called and no change events are fired.
> >> With the next call to beginEvent() the ListEventAssembler detects,
> >> that an event delivery is in progress leading to the
> >> ConcurrentModificationExceptions
> >> you observed.
> >>
> >> Holger
> >> __________________________________________________________________________
> >> Verschicken Sie SMS direkt vom Postfach aus - in alle deutschen und viele
> >> ausländische Netze zum gleichen Preis!
> >> https://produkte.web.de/webde_sms/sms
> >>
> >>
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: users-unsubscribe@...
> >> For additional commands, e-mail: users-help@...
> >>
> >>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: users-unsubscribe@...
> > For additional commands, e-mail: users-help@...
> >
> >
> >
>
> --
> View this message in context: http://www.nabble.com/ConcurrentModificationException%2C-Cannot-begin-a-new-event-...---reproducible-tp23172207p23241485.html
> Sent from the GlazedLists - User mailing list archive at Nabble.com.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@...
> For additional commands, e-mail: users-help@...
>
>

______________________________________________________
GRATIS für alle WEB.DE-Nutzer: Die maxdome Movie-FLAT!
Jetzt freischalten unter http://movieflat.web.de


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@...
For additional commands, e-mail: users-help@...

[James Lemieux]

It's highly unlikely that a pipeline, or any related listeners will be consistent if a RuntimeException occurs while delivering a ListEvent. I think failing hard and failing often is the optimal approach in this case, and if I were to improve the current behaviour, I'd simply make it throw a more appropriate type of Exception with a descriptive message.

The "big reason a commit cannot finish despite the error" is because an EventList in the pipeline failed to build and deliver a proper ListEvent downstream. In fact, it may have only finished reacting to a portion of the ListEvent it was delivered at the time it throw its RuntimeException. This type of inconsistency cannot be tolerated, and it could lead to red-herring types of error reports where subsequent strange behaviour is reported as the bug rather than the root cause: a RuntimeException during the delivery of a *prior* ListEvent.

James

On Sun, Apr 26, 2009 at 4:52 AM, Emil Crumhorn <emil.crumhorn@...> wrote:

I completely agree. It seems to me it should be possible to make a "mistake"
without getting the entire list system out of order. Normal java lists just
continue to work even if you mess up the index (after throwing an exception
of course).

Unless there's a big reason why the commit can't finish despite the error,
it seems to me this would be an easy fix and at least you'd get the lists
back on track. Now I have to explicitly check indexes before a set just so
that I never get an IndexOutOfBounds exception, as if I get one, then I
can't use the lists anymore.

Emil

Endre Stølsvik-8 wrote:
>
> I've been wondering about this: shouldn't the beginEvent-commitEvent
> always be in a try-finally block? This to contain any errors as best
> one can? It seems to me like any error anywhere in the pipeline now
> will lead to an irreparably inconsistent state, while if one had
> try-finally, with the updates.["change"] always after the actual data
> change invocation method, one would get something that at least in
> several situations resembles a transaction with both commit and
> rollback semantics.
>
> This would be so that even though there are some logic errors to could
> come up in certain situations, the application using GL would still
> have a chance of being consistent after an error - of being able to
> recover out of errors.
>
> Endre.
>
> On Sun, Apr 26, 2009 at 11:06, Holger Brands <hbrands@...> wrote:
>>  > Is this the expected behavior?
>>
>> Yes, with the current implementation, that's expected.
>> If you dive into the implementation you'll see this:
>>
>>    public E set(int index, E element) {
>>        // create the change event
>>        updates.beginEvent();
>>        // do the actual set
>>        E previous = data.set(index, element);
>>        // fire the event
>>        updates.elementUpdated(index, previous);
>>        updates.commitEvent();
>>        return previous;
>>    }
>>
>> Event delivery is guarded by calls to
>> ListEventAssembler.beginEvent() - start recording changes
>> and
>> ListEventAssembler.commitEvent() - propagate recorded changes
>>
>> When an uncaught exception is thrown inbetween, commitEvent()
>> is never called and no change events are fired.
>> With the next call to beginEvent() the ListEventAssembler detects,
>> that an event delivery is in progress leading to the
>> ConcurrentModificationExceptions
>> you observed.
>>
>> Holger
>> __________________________________________________________________________
>> Verschicken Sie SMS direkt vom Postfach aus - in alle deutschen und viele
>> ausländische Netze zum gleichen Preis!
>> https://produkte.web.de/webde_sms/sms
>>
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscribe@...
>> For additional commands, e-mail: users-help@...
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@...
> For additional commands, e-mail: users-help@...
>
>
>

--
View this message in context: http://www.nabble.com/ConcurrentModificationException%2C-Cannot-begin-a-new-event-...---reproducible-tp23172207p23241485.html

Sent from the GlazedLists - User mailing list archive at Nabble.com.


---------------------------------------------------------------------

To unsubscribe, e-mail: users-unsubscribe@...
For additional commands, e-mail: users-help@...

[Endre Stølsvik-8]

Well, okay - but I guess in the example Holger pastes, the exception
originates at the point of changing the data - not in the event
processing pipeline. Thus, by always committing (or maybe not start
the transaction until the data is changed - what would be the actual
difference in this case? Locking is done outside..), the error would
be contained fully. I get this hunch that this would more often be the
case than the error cropping up in the pipeline. This because the
pipeline-initiated changes often are more of a
"automatic"/"procedural" nature executed by library code, while the
direct list manipulations aren't.

Also, I don't quite see the argument _against trying_ to contain the
exception. The exception would still be raised (fast fail, of course),
the question here is how contained the error would be. Again, in
Holger's example code, if the user actually invoked set(...) there,
the error would have been fully contained - but the method would
obviously have thrown. If the error instead came from commit() (thus
came from the pipeline updates), no guarantees regarding the pipeline
states could be made at all - but it would in any case not be
_completely_ trashed in the way that the state of the transaction
wasn't finished - and the method would obviously still have thrown.
Thus, in both cases, the user would have been directly notified about
the error - but one would on the other side not have a GUARANTEE that
the application state was completely fucked over, as one now have.

Endre.


On Sun, Apr 26, 2009 at 14:30, James Lemieux <jplemieux@...> wrote:

> It's highly unlikely that a pipeline, or any related listeners will be
> consistent if a RuntimeException occurs while delivering a ListEvent. I
> think failing hard and failing often is the optimal approach in this case,
> and if I were to improve the current behaviour, I'd simply make it throw a
> more appropriate type of Exception with a descriptive message.
>
> The "big reason a commit cannot finish despite the error" is because an
> EventList in the pipeline failed to build and deliver a proper ListEvent
> downstream. In fact, it may have only finished reacting to a portion of the
> ListEvent it was delivered at the time it throw its RuntimeException. This
> type of inconsistency cannot be tolerated, and it could lead to red-herring
> types of error reports where subsequent strange behaviour is reported as the
> bug rather than the root cause: a RuntimeException during the delivery of a
> *prior* ListEvent.
>
> James
>
> On Sun, Apr 26, 2009 at 4:52 AM, Emil Crumhorn <emil.crumhorn@...>
> wrote:
>>
>> I completely agree. It seems to me it should be possible to make a
>> "mistake"
>> without getting the entire list system out of order. Normal java lists
>> just
>> continue to work even if you mess up the index (after throwing an
>> exception
>> of course).
>>
>> Unless there's a big reason why the commit can't finish despite the error,
>> it seems to me this would be an easy fix and at least you'd get the lists
>> back on track. Now I have to explicitly check indexes before a set just so
>> that I never get an IndexOutOfBounds exception, as if I get one, then I
>> can't use the lists anymore.
>>
>> Emil
>>
>>
>> Endre Stølsvik-8 wrote:
>> >
>> > I've been wondering about this: shouldn't the beginEvent-commitEvent
>> > always be in a try-finally block? This to contain any errors as best
>> > one can? It seems to me like any error anywhere in the pipeline now
>> > will lead to an irreparably inconsistent state, while if one had
>> > try-finally, with the updates.["change"] always after the actual data
>> > change invocation method, one would get something that at least in
>> > several situations resembles a transaction with both commit and
>> > rollback semantics.
>> >
>> > This would be so that even though there are some logic errors to could
>> > come up in certain situations, the application using GL would still
>> > have a chance of being consistent after an error - of being able to
>> > recover out of errors.
>> >
>> > Endre.
>> >
>> > On Sun, Apr 26, 2009 at 11:06, Holger Brands <hbrands@...> wrote:
>> >>  > Is this the expected behavior?
>> >>
>> >> Yes, with the current implementation, that's expected.
>> >> If you dive into the implementation you'll see this:
>> >>
>> >>    public E set(int index, E element) {
>> >>        // create the change event
>> >>        updates.beginEvent();
>> >>        // do the actual set
>> >>        E previous = data.set(index, element);
>> >>        // fire the event
>> >>        updates.elementUpdated(index, previous);
>> >>        updates.commitEvent();
>> >>        return previous;
>> >>    }
>> >>
>> >> Event delivery is guarded by calls to
>> >> ListEventAssembler.beginEvent() - start recording changes
>> >> and
>> >> ListEventAssembler.commitEvent() - propagate recorded changes
>> >>
>> >> When an uncaught exception is thrown inbetween, commitEvent()
>> >> is never called and no change events are fired.
>> >> With the next call to beginEvent() the ListEventAssembler detects,
>> >> that an event delivery is in progress leading to the
>> >> ConcurrentModificationExceptions
>> >> you observed.
>> >>
>> >> Holger
>> >>
>> >> __________________________________________________________________________
>> >> Verschicken Sie SMS direkt vom Postfach aus - in alle deutschen und
>> >> viele
>> >> ausländische Netze zum gleichen Preis!
>> >> https://produkte.web.de/webde_sms/sms
>> >>
>> >>
>> >>
>> >>
>> >> ---------------------------------------------------------------------
>> >> To unsubscribe, e-mail: users-unsubscribe@...
>> >> For additional commands, e-mail: users-help@...
>> >>
>> >>
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: users-unsubscribe@...
>> > For additional commands, e-mail: users-help@...
>> >
>> >
>> >
>>
>> --
>> View this message in context:
>> http://www.nabble.com/ConcurrentModificationException%2C-Cannot-begin-a-new-event-...---reproducible-tp23172207p23241485.html
>> Sent from the GlazedLists - User mailing list archive at Nabble.com.
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscribe@...
>> For additional commands, e-mail: users-help@...
>>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@...
For additional commands, e-mail: users-help@...