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

Changes to RGB LED animation thread to prevent a thread leak and properly stop animation #54

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
1 participant
@HyruleWarrior

HyruleWarrior commented Jul 18, 2018

In StartRunningColors() it is possible for there to be a thread leak if it is called multiple times without calling Stop() outside the method (and also waiting at least the animation duration). Stop() would just set _running to false and immediately after it back to true so a currently running animation thread most often wouldn't be at the end of it's while loop at this time, so another thread will kick off and the current one will continue. This causes odd animation behavior and eventually crashes the Netduino. Because of a related race condition, if an animation is stopped and a color is set right after, the color that is set will most often get overridden by the animation thread turning off the LED when it completes.

Changes to animation thread to prevent a thread leak and to make sure
animation doesn't stomp over a set color.

In StartRunningColors() it is possible for there to be a thread leak if it is
called multiple times without calling Stop() outside the method (and also
waiting). Stop() would just set _running to false and immediately after set
it back to true so a currently running animation thread most often wouldn't
be at the end of it's while loop at this time, so another thread will kick
off and the current one will continue. This causes odd animation behavior
and eventually crashes the Netduino. Because of a related race condition,
if an animation is stopped and a color is set right after, the color that
is set will most often get overriden by the animation thread turning off
the LED when it completes.
@HyruleWarrior

This comment has been minimized.

Show comment
Hide comment
@HyruleWarrior

HyruleWarrior Jul 18, 2018

Just noticed the bit about updating dependencies on Netduino.Foundation.Core. I am guessing that should be done in this case? If so, I can update the pull request with the results of running the script.

HyruleWarrior commented Jul 18, 2018

Just noticed the bit about updating dependencies on Netduino.Foundation.Core. I am guessing that should be done in this case? If so, I can update the pull request with the results of running the script.

@HyruleWarrior HyruleWarrior changed the title from Changes to animation thread to prevent a thread leak and properly stop animation to Changes to RGB animation thread to prevent a thread leak and properly stop animation Aug 9, 2018

@HyruleWarrior HyruleWarrior changed the title from Changes to RGB animation thread to prevent a thread leak and properly stop animation to Changes to RGB LED animation thread to prevent a thread leak and properly stop animation Aug 9, 2018

@HyruleWarrior

This comment has been minimized.

Show comment
Hide comment
@HyruleWarrior

HyruleWarrior Aug 22, 2018

A more recent pull request was accepted that resolves the issues above.

HyruleWarrior commented Aug 22, 2018

A more recent pull request was accepted that resolves the issues above.

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