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

Prevent the renderer from hanging if the main model panics during its init #905

Conversation

timmattison
Copy link
Contributor

This should fix #904

The issue arises if the main model panics during init. If this happens it tries to send a signal via r.done. But nothing is reading r.done until go r.listen() is called in r.start().

In r.start() the r.ticker value is set up, then r.once is initialized, and then finally go r.listen() is called. Since r.ticker is only assigned a value in this one place we can be reasonably certain that if it is non-nil that go r.listen() has been called and it is monitoring the r.done channel.

An alternate solution would be to add a new variable to indicate that the system has entered r.listen(). I like that solution better, but I wasn't sure if it makes sense to add another variable for this edge case.

@meowgorithm
Copy link
Member

Hi! Does #926 fix this? If so, would prefer that one as it’s a simpler change.

@timmattison
Copy link
Contributor Author

It looks like it does but I haven't tested it. I prefer that one too if it works.

@meowgorithm
Copy link
Member

Just checked and it appears to. Speaking of which, thank you for the very well documented issue; made it very easy to vet.

@maaslalani
Copy link
Contributor

Closing this PR as it seems that #926 solves the issue.

Please feel free to re-open this PR if #926 doesn't solve #904 / #924 after it's merged.

@maaslalani maaslalani closed this Mar 2, 2024
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.

Application hang when calling panic
3 participants