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

panic: "The Promise Sender was dropped" #25

Open
emilk opened this issue Jan 19, 2024 · 0 comments
Open

panic: "The Promise Sender was dropped" #25

emilk opened this issue Jan 19, 2024 · 0 comments
Labels
enhancement New feature or request

Comments

@emilk
Copy link
Collaborator

emilk commented Jan 19, 2024

poll-promise panics if the user drops the Sender before sending a value.

This is quite a footgun.

A dropped sender is a bug (because we cannot deliver a value), but I'm not sure it deserves a crash.

We could change the interface of Promise to return a Result, with an error for "DroppedSender".

We could also consider having the panic only in debug builds, and in release builds treat a dropped receiver the same as a receiver that hasn't sent anything yet, i.e. the promise will be stuck in "Pending" forever. Perhaps with a logged warning.

@emilk emilk added the enhancement New feature or request label Jan 19, 2024
emilk added a commit to rerun-io/rerun that referenced this issue Jan 19, 2024
### What
* Introduced in #4841

`poll-promise` has a gotcha: if you drop the `Sender` without sending a
result,, the `Promise` will panic when polled.

I opened an issue about this:
EmbarkStudios/poll-promise#25

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using newly built examples:
[app.rerun.io](https://app.rerun.io/pr/4872/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/4872/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/4872/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4872)
- [Docs
preview](https://rerun.io/preview/bdbdc1f71719e792869be347ebef16bcb318f2dd/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/bdbdc1f71719e792869be347ebef16bcb318f2dd/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant