Skip to content

Refactor AsyncWrapper to make it safer#1206

Merged
COM8 merged 4 commits intolibcpr:masterfrom
Anohkka:async-wrapper-refactor
Apr 13, 2025
Merged

Refactor AsyncWrapper to make it safer#1206
COM8 merged 4 commits intolibcpr:masterfrom
Anohkka:async-wrapper-refactor

Conversation

@Anohkka
Copy link
Contributor

@Anohkka Anohkka commented Mar 30, 2025

It's currently possible to construct an unsafe cancelable AsyncWrapper by explicitly specifying the template arguments in the constructor, like so:

#include "cpr/cpr.h"

int main(int, char**) {
    auto wrapper = cpr::AsyncWrapper<void, true>{std::future<void>{}};
    
    // Dereferences a null pointer
    auto _ = wrapper.IsCancelled();
}

To prevent that, we split AsyncWrapper it into two template specializations. AsyncWrapper<T, false> keeps most of the code, while cancelation-related code is made exclusive to AsyncWrapper<T, true>. Most importantly, we can prevent a cancelable AsyncWrapper from being constructed without a provided pointer. AsyncWrappers that aren't cancelable get to be a bit slimmer, no longer containing the pointer to keep track of cancelation state.

Anohkka added 4 commits March 30, 2025 16:02
AsyncWrapper<T, false> no longer has an isCancelled() member function
It creates an unsafe object, and tests pass without it
@Anohkka Anohkka changed the title Refactor AsyncWrapper to make it a bit safer Refactor AsyncWrapper to make it safer Mar 31, 2025
@COM8 COM8 assigned COM8 and Anohkka and unassigned COM8 Apr 13, 2025
Copy link
Member

@COM8 COM8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@COM8 COM8 merged commit 29cd461 into libcpr:master Apr 13, 2025
44 checks passed
@Anohkka Anohkka deleted the async-wrapper-refactor branch April 17, 2025 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants