Skip to content

Conversation

@dsrees
Copy link
Owner

@dsrees dsrees commented Aug 8, 2019

There are still a few ConcurrentModificationExceptions occurring. These are caused by clients adding a callback hook inside of a callback hook or modifying the channels while handling an error, etc.

the solution is to make lists as read-only and then whenever adding or removing hooks and channels, use only concurrently safe operations by converting the read-only list to a new mutable list, adding the value, and then returning the new list.

This allows for looping over an existing list and adding a value to that list at the same time without modifying the underlying list while it's being looped over.

@dsrees
Copy link
Owner Author

dsrees commented Aug 8, 2019

@AdamGrzybkowski

@codecov-io
Copy link

codecov-io commented Aug 8, 2019

Codecov Report

Merging #70 into master will increase coverage by 0.06%.
The diff coverage is 95.83%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #70      +/-   ##
============================================
+ Coverage     91.36%   91.43%   +0.06%     
  Complexity      178      178              
============================================
  Files             9        9              
  Lines           533      537       +4     
  Branches         72       71       -1     
============================================
+ Hits            487      491       +4     
+ Misses           13       12       -1     
- Partials         33       34       +1
Impacted Files Coverage Δ Complexity Δ
src/main/kotlin/org/phoenixframework/Socket.kt 92.35% <100%> (-0.33%) 71 <5> (ø)
src/main/kotlin/org/phoenixframework/Push.kt 83.33% <50%> (-0.67%) 25 <0> (ø)
.../main/kotlin/org/phoenixframework/DispatchQueue.kt 66.66% <0%> (+11.11%) 0% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d5c52f...ec430fb. Read the comment docs.

@dsrees dsrees merged commit 3a84464 into master Aug 8, 2019
@dsrees dsrees deleted the dr/fix-concurrency-on-callbacks branch August 8, 2019 13:58
@AdamGrzybkowski
Copy link
Contributor

@dsrees I don't think this extension copyAndAdd is needed

converting the read-only list to a new mutable list, adding the value, and then returning the new list

this could be achieved with kotlin plus operator. I've left a comment in the commit to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants