Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # 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