Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix deadlock at startup #291

Merged
merged 4 commits into from Aug 8, 2019
Merged

Fix deadlock at startup #291

merged 4 commits into from Aug 8, 2019

Conversation

igorpisarev
Copy link
Contributor

Closes #239
The cause for the deadlock was that the list of sources in ViewerState was modified under the lock, and since it was an ObservableList, it immediately notified the listeners about the change while still holding the lock.

I've changed it to a regular list and ViewerState to implement Observable instead. It will fire the invalidated event after releasing the lock which eliminates the risk of running into a deadlock.
For consistency it will also fire when the viewer transform or the timepoint changes. I also slightly changed types and visibility of the class members in ViewerState to make it more straightforward.

@hanslovsky
Copy link
Collaborator

hanslovsky commented Aug 7, 2019

Great, thanks @igorpisarev

I will have a look soon and will merge quickly so we can have a timely patch release with this. I will also add a [BUGFIX] tag to the commit message because I am trying to automate the release notes from merge commits.

Copy link
Collaborator

@hanslovsky hanslovsky left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@hanslovsky hanslovsky merged commit 140e602 into master Aug 8, 2019
@igorpisarev igorpisarev deleted the fix-deadlock-at-startup branch August 9, 2019 19:51
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.

0.14.1 breaks Lauritzen projects with synapse rendering
2 participants