Skip to content

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
@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