We are experiencing a java.util.ConcurrentModificationException using gwt 2.8.0, while calling iter.next() in the GWT emulated AbstractHashMap class (See stack trace and our CallbackTimer class below). The lowest point in the trace involving our code is on line 118, in the method private void tick(), a call to iter.next().
Drilling into trace, I see AbstractHashMap:
@Override
public Entry<K, V> next() {
checkStructuralChange(AbstractHashMap.this, this);
checkElement(hasNext());
last = current;
Entry<K, V> rv = current.next();
hasNext = computeHasNext();
return rv;
}
calling ConcurrentModificationDetector.checkStructuralChange:
public static void checkStructuralChange(Object host, Iterator<?> iterator) {
if (!API_CHECK) {
return;
}
if (JsUtils.getIntProperty(iterator, MOD_COUNT_PROPERTY)
!= JsUtils.getIntProperty(host, MOD_COUNT_PROPERTY)) {
throw new ConcurrentModificationException();
}
}
My understanding of the purpose of ConcurrentModificationException is to avoid having the collection change while it is being iterated over. I don't think iter.next() would fall into that category. Further, the only places I see the collection changing during iteration do so through the iterator itself. Am I missing something here? Any help would be appreciated!
Our Stack Trace:
java.util.ConcurrentModificationException
at Unknown.Throwable_1_g$(Throwable.java:61)
at Unknown.Exception_1_g$(Exception.java:25)
at Unknown.RuntimeException_1_g$(RuntimeException.java:25)
at Unknown.ConcurrentModificationException_1_g$(ConcurrentModificationException.java:25)
at Unknown.checkStructuralChange_0_g$(ConcurrentModificationDetector.java:54)
at Unknown.next_79_g$(AbstractHashMap.java:106)
at Unknown.next_78_g$(AbstractHashMap.java:105)
at Unknown.next_81_g$(AbstractMap.java:217)
at Unknown.tick_0_g$(CallbackTimer.java:118)
at Unknown.run_47_g$(CallbackTimer.java:41)
at Unknown.fire_0_g$(Timer.java:135)
at Unknown.anonymous(Timer.java:139)
at Unknown.apply_65_g$(Impl.java:239)
at Unknown.entry0_0_g$(Impl.java:291)
at Unknown.anonymous(Impl.java:77)
The source code for CallbackTimer.java is here:
package com.XXXXX.common.gwt.timer;
import java.util.ConcurrentModificationException;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.logging.Level;
import java.util.logging.Logger;
import com.google.common.base.Optional;
import com.google.gwt.user.client.Timer;
/**
* A {@link Timer} wrapper which allows for the registration of callbacks to be invoked after a given number of ticks.
* The timer will only run if at least one {@link TickCallback} is currently registered and will stop running when all
* callbacks have been unregistered.
*
* The intent of this class is to reduce overhead by allowing all callbacks in a GWT application to use the same
* Javascript timer.
*/
public class CallbackTimer
{
private static final Logger LOGGER = Logger.getLogger(CallbackTimer.class.getName());
private static final int MILLIS_IN_SEC = 1000;
private Timer timer;
private Map<Object, TickCallback> callbackRegistry = new HashMap<>();
public CallbackTimer()
{
timer = new Timer()
{
@Override
public void run()
{
try
{
tick();
}
catch(ConcurrentModificationException concurrentModificationException)
{
LOGGER.log(Level.WARNING, "Concurrent Modification Exception in " +
"CallbackTimer.tick()", concurrentModificationException);
}
}
};
}
public void registerCallback(Object key, TickCallback callback)
{
if (callbackRegistry.containsKey(key))
{
LOGGER.fine("Key " + key.toString() + " is being overwritten with a new callback.");
}
callbackRegistry.put(key, callback);
callback.markStartTime();
LOGGER.finer("Key " + key.toString() + " registered.");
if (!timer.isRunning())
{
startTimer();
}
}
public void unregisterCallback(Object key)
{
if (callbackRegistry.containsKey(key))
{
callbackRegistry.remove(key);
LOGGER.finer("Key " + key.toString() + " unregistered.");
if (callbackRegistry.isEmpty())
{
stopTimer();
}
}
else
{
LOGGER.info("Attempted to unregister key " + key.toString() + ", but this key has not been registered.");
}
}
private void unregisterCallback(Iterator<Object> iter, Object key)
{
iter.remove();
LOGGER.finer("Key " + key.toString() + " unregistered.");
if (callbackRegistry.isEmpty())
{
stopTimer();
}
}
public boolean keyIsRegistered(Object key)
{
return callbackRegistry.containsKey(key);
}
public TickCallback getCallback(Object key)
{
if (keyIsRegistered(key))
{
return callbackRegistry.get(key);
}
else
{
LOGGER.fine("Key " + key.toString() + " is not registered; returning null.");
return null;
}
}
private void tick()
{
long fireTimeMillis = System.currentTimeMillis();
Iterator<Object> iter = callbackRegistry.keySet().iterator();
while (iter.hasNext())
{
Object key = iter.next();//Lowest point in stack for our code
TickCallback callback = callbackRegistry.get(key);
if (callback.isFireTime(fireTimeMillis))
{
if (Level.FINEST.equals(LOGGER.getLevel()))
{
LOGGER.finest("Firing callback for key " + key.toString());
}
callback.onTick();
callback.markLastFireTime();
}
if (callback.shouldTerminate())
{
LOGGER.finer("Callback for key " + key.toString() +
" has reached its specified run-for-seconds and will now be unregistered.");
unregisterCallback(iter, key);
}
}
}
private void startTimer()
{
timer.scheduleRepeating(MILLIS_IN_SEC);
LOGGER.finer(this + " started.");
}
private void stopTimer()
{
timer.cancel();
LOGGER.finer(this + " stopped.");
}
/**
* A task to run on a given interval, with the option to specify a maximum number of seconds to run.
*/
public static abstract class TickCallback
{
private long intervalMillis;
private long startedAtMillis;
private long millisRunningAtLastFire;
private Optional<Long> runForMillis;
/**
* @param intervalSeconds
* The number of seconds which must elapse between each invocation of {@link #onTick()}.
* @param runForSeconds
* An optional maximum number of seconds to run for, after which the TickCallback will be eligible
* to be automatically unregistered. Pass {@link Optional#absent()} to specify that the TickCallback
* must be manually unregistered. Make this value the same as {@param intervalSeconds} to run the
* callback only once.
*/
public TickCallback(int intervalSeconds, Optional<Integer> runForSeconds)
{
this.intervalMillis = intervalSeconds * MILLIS_IN_SEC;
this.runForMillis = runForSeconds.isPresent() ?
Optional.of((long)runForSeconds.get() * MILLIS_IN_SEC) : Optional.<Long>absent();
}
private void markStartTime()
{
millisRunningAtLastFire = 0;
startedAtMillis = System.currentTimeMillis();
}
private void markLastFireTime()
{
millisRunningAtLastFire += intervalMillis;
}
private boolean isFireTime(long nowMillis)
{
return nowMillis - (startedAtMillis + millisRunningAtLastFire) >= intervalMillis;
}
private boolean shouldTerminate()
{
return runForMillis.isPresent() && System.currentTimeMillis() - startedAtMillis >= runForMillis.get();
}
/**
* A callback to be run every time intervalSeconds seconds have past since this callback was registered.
*/
public abstract void onTick();
}
}
Update 2017-06-08
I ended up following walen's first suggestion. I did not see where SimpleEventBus was the right tool for this specific job. I did, however, shamelessly steal SEBs method of integrating newly added/removed callbacks:
package com.XXXXX.common.gwt.timer;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.logging.Level;
import java.util.logging.Logger;
import com.google.common.base.Optional;
import com.google.gwt.user.client.Command;
import com.google.gwt.user.client.Timer;
/**
* A {@link Timer} wrapper which allows for the registration of callbacks to be invoked after a given number of ticks.
* The timer will only run if at least one {@link TickCallback} is currently registered and will stop running when all
* callbacks have been unregistered.
*
* The intent of this class is to reduce overhead by allowing all callbacks in a GWT application to use the same
* Javascript timer.
*/
public class CallbackTimer
{
private static final Logger LOGGER = Logger.getLogger(CallbackTimer.class.getName());
private static final int MILLIS_IN_SEC = 1000;
private Timer timer;
private Map<Object, TickCallback> callbackRegistry = new HashMap<>();
private List<Command> deferredDeltas = new ArrayList<>();
public CallbackTimer()
{
timer = new Timer()
{
@Override
public void run()
{
tick();
}
};
}
public void registerCallback(final Object key, final TickCallback callback)
{
deferredDeltas.add(new Command()
{
@Override
public void execute()
{
activateCallback(key, callback);
}
});
if (!timer.isRunning())
{
startTimer();
}
}
private void activateCallback(Object key, TickCallback callback)
{
if (callbackRegistry.containsKey(key))
{
LOGGER.fine("Key " + key.toString() + " is being overwritten with a new callback.");
}
callbackRegistry.put(key, callback);
callback.markStartTime();
LOGGER.finer("Key " + key.toString() + " registered.");
}
public void unregisterCallback(final Object key)
{
deferredDeltas.add(new Command()
{
@Override
public void execute()
{
deactivateCallback(key);
}
});
}
private void deactivateCallback(Object key)
{
if (callbackRegistry.containsKey(key))
{
callbackRegistry.remove(key);
LOGGER.fine("Key " + key.toString() + " unregistered.");
if (callbackRegistry.isEmpty())
{
stopTimer();
}
}
else
{
LOGGER.info("Attempted to unregister key " + key.toString() + ", but this key has not been registered.");
}
}
private void handleQueuedAddsAndRemoves()
{
for (Command c : deferredDeltas)
{
c.execute();
}
deferredDeltas.clear();
}
public boolean keyIsRegistered(Object key)
{
return callbackRegistry.containsKey(key);
}
private void tick()
{
handleQueuedAddsAndRemoves();
long fireTimeMillis = System.currentTimeMillis();
for (Map.Entry<Object, TickCallback> objectTickCallbackEntry : callbackRegistry.entrySet())
{
Object key = objectTickCallbackEntry.getKey();
TickCallback callback = objectTickCallbackEntry.getValue();
if (callback.isFireTime(fireTimeMillis))
{
if (Level.FINEST.equals(LOGGER.getLevel()))
{
LOGGER.finest("Firing callback for key " + key.toString());
}
callback.onTick();
callback.markLastFireTime();
}
if (callback.shouldTerminate())
{
LOGGER.finer("Callback for key " + key.toString() +
" has reached its specified run-for-seconds and will now be unregistered.");
unregisterCallback(key);
}
}
}
private void startTimer()
{
timer.scheduleRepeating(MILLIS_IN_SEC);
LOGGER.finer(this + " started.");
}
private void stopTimer()
{
timer.cancel();
LOGGER.finer(this + " stopped.");
}
/**
* A task to run on a given interval, with the option to specify a maximum number of seconds to run.
*/
public static abstract class TickCallback
{
private long intervalMillis;
private long startedAtMillis;
private long millisRunningAtLastFire;
private Optional<Long> runForMillis;
/**
* @param intervalSeconds The number of seconds which must elapse between each invocation of {@link #onTick()}.
* @param runForSeconds An optional maximum number of seconds to run for, after which the TickCallback will be
* eligible
* to be automatically unregistered. Pass {@link Optional#absent()} to specify that the TickCallback
* must be manually unregistered. Make this value the same as {@param intervalSeconds} to run the
* callback only once.
*/
protected TickCallback(int intervalSeconds, Optional<Integer> runForSeconds)
{
this.intervalMillis = intervalSeconds * MILLIS_IN_SEC;
this.runForMillis = runForSeconds.isPresent() ?
Optional.of((long) runForSeconds.get() * MILLIS_IN_SEC) : Optional.<Long>absent();
}
private void markStartTime()
{
millisRunningAtLastFire = 0;
startedAtMillis = System.currentTimeMillis();
}
private void markLastFireTime()
{
millisRunningAtLastFire += intervalMillis;
}
private boolean isFireTime(long nowMillis)
{
return nowMillis - (startedAtMillis + millisRunningAtLastFire) >= intervalMillis;
}
private boolean shouldTerminate()
{
return runForMillis.isPresent() && System.currentTimeMillis() - startedAtMillis >= runForMillis.get();
}
/**
* A callback to be run every time intervalSeconds seconds have past since this callback was registered.
*/
public abstract void onTick();
}
}
