Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

Scheduler: fix ConcurrentModificationException #3511

Merged
merged 1 commit into from
May 26, 2017

Conversation

sjsf
Copy link
Contributor

@sjsf sjsf commented May 24, 2017

... by expanding the synchronized block to include the outer loop.

And while being at it anyway I also vaporized the removal logic to a single line.

fixes #3504
Signed-off-by: Simon Kaufmann simon.kfm@googlemail.com

fixes eclipse-archived#3504
Signed-off-by: Simon Kaufmann <simon.kfm@googlemail.com>
futures.get(aRunnable).remove(toDelete);
}
synchronized (futures) {
for (Runnable aRunnable : futures.keySet()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entrySet() in map is faster than keySet()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True for HashMap. However,

for (Entry<Runnable, ArrayList<Future<?>>> entry : futures.entrySet()) {
    futures.get(entry.getKey()).removeIf(future -> future == runnable);
}

is also less readable. Therefore I thought I leave it as it is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree - readability is as important as efficiency. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...at least for small collections like this 😄

@maggu2810 maggu2810 merged commit d4968f8 into eclipse-archived:master May 26, 2017
cdjackson added a commit to cdjackson/smarthome that referenced this pull request Jun 3, 2017
* master: (335 commits)
  Voice: send feedback to an item when listening for a command (eclipse-archived#3451)
  Scheduler: fix ConcurrentModificationException (eclipse-archived#3511)
  [Scheduler] Dropped usage of SimpleDateFormat for trace logging (eclipse-archived#3508)
  Use % sign as default for dimmer items (eclipse-archived#3525)
  updated ThingHandler javadoc
  added regression tests
  ensure thingUdpated cannot be called in parallel with initialize
  moving handler intiialization back to the caller thread
  use decicated locks instead of syncrhonization by object
  add PaperUI setup to IDE setup tasks, adopt documentation (eclipse-archived#3515)
  Bug fix: things not showing on control page (eclipse-archived#3517)
  remove "Type" postfix in item events, fixes 3282. (eclipse-archived#3494)
  use toFullString when creating GroupItemStateChangeEvent (eclipse-archived#3409)
  Made item’s state text consistent + refactored code (eclipse-archived#3466)
  Refactored CoreItemFactory: (eclipse-archived#3507)
  Add CoreItemFactoryTest, in addition to eclipse-archived#3507 (eclipse-archived#3509)
  fix ConcurrentModificationException in AbstractWatchServiceTest (eclipse-archived#3499)
  BasicUI: Treat Switch on NumberItem not as ON/OFF Switch (eclipse-archived#3493)
  An Interface method should not allow for throwing a generic Exception (eclipse-archived#3467)
  Fix eclipse-archived#2902 for Basic UI (eclipse-archived#3496)
  ...

# Conflicts:
#	bundles/io/org.eclipse.smarthome.io.transport.dbus/.classpath
#	bundles/io/org.eclipse.smarthome.io.transport.dbus/META-INF/MANIFEST.MF
#	extensions/binding/pom.xml
@kaikreuzer kaikreuzer modified the milestone: 0.9.0 Jun 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Scheduler] ConcurrentModificationException in ExpressionThreadPoolManager
4 participants