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
Merged

Scheduler: fix ConcurrentModificationException #3511

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 3 additions & 14 deletions ....core/src/main/java/org/eclipse/smarthome/core/scheduler/ExpressionThreadPoolManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,20 +105,9 @@ protected void afterExecute(Runnable runnable, Throwable throwable) {
super.afterExecute(runnable, throwable);

if (runnable instanceof Future) {
for (Runnable aRunnable : futures.keySet()) {
Future<?> toDelete = null;
synchronized (futures) {
for (Future<?> future : futures.get(aRunnable)) {
if (future == runnable) {
toDelete = future;
break;
}
}
if (toDelete != null) {
logger.trace("Removing Future '{}' (out of {}) for Runnable '{}'", toDelete,
futures.get(aRunnable).size(), aRunnable);
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 😄

futures.get(aRunnable).removeIf(future -> future == runnable);
}
}

Expand Down