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

Safeguard for Ticker internal storage that may be changed during callback execution #8820

Merged
merged 4 commits into from
Jan 26, 2023

Conversation

mcspr
Copy link
Collaborator

@mcspr mcspr commented Jan 20, 2023

Mainly, we protect std::function and variant itself from undefined behaviour when the underlying object is destroyed and then re-created with something else inside of it.

For example, every captured object would be destroyed.

Ticker ticker;
void oops(Job job) {
    ticker.once(1.0f,
        [job = std::move(job)]() {
            ticker.once(job.next(), do_something_else);
            job.work(); // ~Job() just happened!
        });
}

Another important case is repeating through once() (since why not)

Ticker ticker;
void repeat() {
    ticker.once(1.0f, repeat);
    puts("tick");
}

void setup() {
    repeat();
}

Using temporary variable in the static_callback func and returning earlier should solve both cases.

Mainly, we protect std::function and variant itself from undefined
behaviour when the underlying object is destroyed and then re-created
with something else inside of it.

For example, every captured object would be destroyed.
```cpp
Ticker ticker;
void oops(Job job) {
    ticker.once(1.0f,
        [job = std::move(job)]() {
            ticker.once(job.next(), do_something_else);
            job.work(); // ~Job() just happened!
        });
}
```

Another important case is repeating through once()
```cpp
Ticker ticker;
void repeat() {
    ticker.once(1.0f, repeat);
    puts("tick");
}

void setup() {
    repeat();
}
```

Temporarily moving callback object into the function and returning
earlier should solve both cases.
@d-a-v d-a-v added the alpha included in alpha release label Jan 24, 2023
@d-a-v d-a-v added this to the 3.1.2 milestone Jan 26, 2023
@mcspr mcspr changed the title Ticker callback should be allowed to be destroyed Safeguard for Ticker internal storage that may be changed during callback execution Jan 26, 2023
@mcspr mcspr merged commit e25f9e9 into esp8266:master Jan 26, 2023
@mcspr mcspr deleted the ticker/undefined branch January 26, 2023 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alpha included in alpha release component: libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants