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 accessing destroyed objects in the callback of async_wait #362

Merged
merged 3 commits into from
Dec 6, 2023

Conversation

BewareMyPower
Copy link
Contributor

Fixes #358 Fixes #359

Motivation

async_wait is not used correctly in some places. A callback that captures the this pointer or reference to this is passed to async_wait, if this object is destroyed when the callback is called, an invalid memory access will happen.

Modifications

Use the following pattern in all async_wait calls.

std::weak_ptr<T> weakSelf{shared_from_this()};
timer_->async_wait([weakSelf](/* ... */) {
    if (auto self = weakSelf.lock()) {
        self->foo();
    }
});

@BewareMyPower BewareMyPower added the bug Something isn't working label Dec 4, 2023
@BewareMyPower BewareMyPower added this to the 3.5.0 milestone Dec 4, 2023
@BewareMyPower BewareMyPower self-assigned this Dec 4, 2023
Fixes apache#358
Fixes apache#359

### Motivation

`async_wait` is not used correctly in some places. A callback that
captures the `this` pointer or reference to `this` is passed to
`async_wait`, if this object is destroyed when the callback is called,
an invalid memory access will happen.

### Modifications

Use the following pattern in all `async_wait` calls.

```c++
std::weak_ptr<T> weakSelf{shared_from_this()};
timer_->async_wait([weakSelf](/* ... */) {
    if (auto self = weakSelf.lock()) {
        self->foo();
    }
});
```
@BewareMyPower BewareMyPower force-pushed the bewaremypower/safe-timer branch from c093610 to 0729a9e Compare December 4, 2023 13:11
@BewareMyPower BewareMyPower marked this pull request as draft December 5, 2023 02:06
@BewareMyPower
Copy link
Contributor Author

It seems some tests failed. I will fix them ASAP.

@BewareMyPower BewareMyPower marked this pull request as ready for review December 5, 2023 03:44
@BewareMyPower BewareMyPower marked this pull request as draft December 5, 2023 05:43
@BewareMyPower BewareMyPower marked this pull request as ready for review December 5, 2023 05:58
@BewareMyPower BewareMyPower merged commit 24ab12c into apache:main Dec 6, 2023
12 checks passed
@BewareMyPower BewareMyPower deleted the bewaremypower/safe-timer branch December 6, 2023 02:39
BewareMyPower added a commit that referenced this pull request Dec 6, 2023
Fixes #358
Fixes #359

### Motivation

`async_wait` is not used correctly in some places. A callback that
captures the `this` pointer or reference to `this` is passed to
`async_wait`, if this object is destroyed when the callback is called,
an invalid memory access will happen.

### Modifications

Use the following pattern in all `async_wait` calls.

```c++
std::weak_ptr<T> weakSelf{shared_from_this()};
timer_->async_wait([weakSelf](/* ... */) {
    if (auto self = weakSelf.lock()) {
        self->foo();
    }
});
```

(cherry picked from commit 24ab12c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working release/3.4.2
Projects
None yet
2 participants