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: make handle_unilateral non-async #114

Merged
merged 1 commit into from
Feb 3, 2025
Merged

Conversation

link2xt
Copy link
Contributor

@link2xt link2xt commented Jan 28, 2025

The check for unsolicited.is_full()
at the beginning of handle_unilateral
is not sufficient if the function is called
from multiple threads parallel.
This normally should not happen,
but not guaranteed.

Instead of checking if the channel is full in advance, use tr_send and ignore the error
if the channel happens to be full
when we try to send into it.

We also ignore the error when the channel
is closed instead of panic
because the library should never panic.

@link2xt
Copy link
Contributor Author

link2xt commented Jan 28, 2025

I am looking into the code of IDLE implementation because of deltachat/deltachat-core-rust#6477

@link2xt link2xt force-pushed the link2xt/handle_unilateral branch from 2c2309c to 73a5a1d Compare January 28, 2025 22:36
The check for `unsolicited.is_full()`
at the beginning of `handle_unilateral`
is not sufficient if the function is called
from multiple threads parallel.
This normally should not happen,
but not guaranteed.

Instead of checking if the channel is full in advance,
use `tr_send` and ignore the error
if the channel happens to be full
when we try to send into it.

We also ignore the error when the channel
is closed instead of panic
because the library should never panic.
@link2xt link2xt force-pushed the link2xt/handle_unilateral branch from 73a5a1d to af55344 Compare January 28, 2025 22:43
@link2xt link2xt merged commit 37ebc2c into main Feb 3, 2025
22 checks passed
@link2xt link2xt deleted the link2xt/handle_unilateral branch February 3, 2025 09:57
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