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

Finish futures when their promise is broken #187

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

jb-gcx
Copy link
Contributor

@jb-gcx jb-gcx commented Aug 12, 2024

Accidentally discarding a djinni::Promise leads the associated future(s) to block forever in get. This is unrecoverable when it happens. Instead, I'm applying the behavior of std::promise::~promise to djinni::Promise: if the promise is not finished, we set a BrokenPromiseException into the shared state.

This is (kind of) a breaking change. But it only changes behavior for users, who rely on futures that never complete when the promise is discarded. In my opinion, that was buggy behavior.

Copy link
Contributor

@li-feng-sc li-feng-sc left a comment

Choose a reason for hiding this comment

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

Thank you!

@li-feng-sc li-feng-sc merged commit 971228b into Snapchat:main Aug 14, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants