Skip to content

Synchronize notifications list access#254

Merged
itdelatrisu merged 2 commits intoitdelatrisu:masterfrom
tpenguinltg:synchronized-notifications
Feb 6, 2017
Merged

Synchronize notifications list access#254
itdelatrisu merged 2 commits intoitdelatrisu:masterfrom
tpenguinltg:synchronized-notifications

Conversation

@tpenguinltg
Copy link
Copy Markdown
Contributor

@tpenguinltg tpenguinltg force-pushed the synchronized-notifications branch from 433734f to a63dedb Compare February 6, 2017 14:55
Prevents a ConcurrentModificationException when a notification is added
while the list is iterated over.

This approach is recommended in the docs:
https://docs.oracle.com/javase/7/docs/api/java/util/Collections.html#synchronizedList%28java.util.List%29
@tpenguinltg tpenguinltg force-pushed the synchronized-notifications branch from a63dedb to 1f6c9df Compare February 6, 2017 14:56
Clearing the notifications is actually the end part of the larger
operation done by the preceding for loop, so it should be in the same
synchronized block. Doing this prevents a race condition where a
notification is added, but is then immediately removed without being
shown.
@itdelatrisu
Copy link
Copy Markdown
Owner

Thanks, my bad.

@itdelatrisu itdelatrisu merged commit 73cdde6 into itdelatrisu:master Feb 6, 2017
@itdelatrisu itdelatrisu added the bug label Feb 6, 2017
@tpenguinltg tpenguinltg deleted the synchronized-notifications branch February 6, 2017 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants