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

Unexpected behavior of cyclic signals #120

Open
mode89 opened this issue Jul 26, 2024 · 2 comments
Open

Unexpected behavior of cyclic signals #120

mode89 opened this issue Jul 26, 2024 · 2 comments

Comments

@mode89
Copy link

mode89 commented Jul 26, 2024

If a cyclic signal goes out of scope, other nodes (that are implemented in terms of the cyclic signal) don't behave as expected.

Here is an example that works as expected:

let sink = Sink::new();
let stream = sink.stream();

let mut start = Stream::never();
let mut stop = Stream::never();

let in_process = Signal::cyclic(|in_process| {
    start = in_process
        .snapshot(&stream, |in_process, _|
            if in_process { None } else { Some(()) })
        .filter_some();
    stop = in_process
        .snapshot(&stream, |in_process, _|
            if in_process { Some(()) } else { None })
        .filter_some();
    start.map(|_| true)
        .merge(&stop.map(|_| false))
        .hold(false)
});

let mut events = start.map(|_| 0).merge(&stop.map(|_| 1)).events();

sink.send(());
assert_eq!(events.next(), Some(0));
sink.send(());
assert_eq!(events.next(), Some(1));
sink.send(());
assert_eq!(events.next(), Some(0));

But if we limit lifetime of the cyclic signal, the test breaks on the second assertion:

let sink = Sink::new();
let stream = sink.stream();

let mut start = Stream::never();
let mut stop = Stream::never();

{
    let in_process = Signal::cyclic(|in_process| {
        start = in_process
            .snapshot(&stream, |in_process, _|
                if in_process { None } else { Some(()) })
            .filter_some();
        stop = in_process
            .snapshot(&stream, |in_process, _|
                if in_process { Some(()) } else { None })
            .filter_some();
        start.map(|_| true)
            .merge(&stop.map(|_| false))
            .hold(false)
    });
}

let mut events = start.map(|_| 0).merge(&stop.map(|_| 1)).events();

sink.send(());
assert_eq!(events.next(), Some(0));
sink.send(());
assert_eq!(events.next(), Some(1));
sink.send(());
assert_eq!(events.next(), Some(0));

By the way, great library - ergonomics are really nice :)

@mode89
Copy link
Author

mode89 commented Jul 26, 2024

After giving a look to the source code, it seems that start and stop depend on SignalCycle::signal version of in_process. And SignalCycle::signal doesn't track value returned by Signal::cyclic (which is also the value returned by the function passed into Signal::cyclic) as it's dependency, so when that value dropped, SignalCycle::signal (as well as start and stop) doesn't get any updates.

Please correct me if I'm wrong.

@milibopp
Copy link
Owner

It has been a while since I touched this code, so I definitely need some time to get back into it. Anyway, a fix with a regression test case and PR would be appreciated.

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

No branches or pull requests

2 participants