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 race condition when stopping blinking #49

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

smortime
Copy link

@smortime smortime commented Jan 5, 2024

Was running into couple different race conditions when calling blink then trying to set the LED back to on. Minimal program to recreate bug :

use std::{thread, time::Duration};
use rust_gpiozero::LED;

fn main() {
    let mut led = LED::new(12);
    led.on();
    thread::sleep(Duration::from_secs(5));
    led.blink(3, 3);
    thread::sleep(Duration::from_secs(5));
    led.on();
    thread::sleep(Duration::from_secs(5));
    led.off();
}

Two race conditions I was hitting:

  1. led.on() called while blinker is running; it sets the blinking to false then turns the device to on but by the time blinker wakes from sleep the device is on and it then turns it off again before bailing from its loop
  2. If you call led.on() between the on/off sleep interval the blinker will then turn it off

The first is remediated by removing device.lock().unwrap().off(); from the if !blinking.load(Ordering::SeqCst){ .. } condition and just relying on who ever calls self.stop() to set the final state it wants.

The second remediated by adding the if !blinking.load(Ordering::SeqCst){ .. } check between sleeps to avoid clobbering the state whoever called stop set (feel there might be cleaner way to do this part 😅).

@@ -188,7 +192,6 @@ macro_rules! impl_digital_output_device {

fn stop(&self) {
self.blinking.clone().store(false, Ordering::SeqCst);
self.device.lock().unwrap().pin.set_low();
Copy link
Author

Choose a reason for hiding this comment

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

I removed this since the only callers are (on/off) and they set the final state they want anyways so seems unnecessary but I can add it back.

@smortime smortime changed the title Add fix race condition when stopping blinking Fix race condition when stopping blinking Jan 5, 2024
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.

1 participant