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

Switch to an async mutex for handshake nonces #2031

Merged
merged 1 commit into from
Apr 21, 2021

Conversation

teor2345
Copy link
Contributor

Motivation

In the handshake code, we use a threaded mutex, which blocks all the tasks on the thread when we're waiting for a lock.

Solution

Instead, use an async mutex, which passes the thread to another task while it is waiting.

The code in this pull request has:

  • Documentation Comments
  • Integration Tests

Review

@dconnolly can review after #2008 merges. This fix isn't urgent, but we do need to clear some of the PR backlog to avoid conflicts.

Related Issues

Closes #1995 (the error slot and address book need a threaded mutex for now)

@teor2345 teor2345 added C-bug Category: This is a bug A-rust Area: Updates to Rust code C-cleanup Category: This is a cleanup P-Medium C-security Category: Security issues I-hang A Zebra component stops responding to requests I-slow Problems with performance or responsiveness labels Apr 19, 2021
@teor2345 teor2345 added this to the 2021 Sprint 7 milestone Apr 19, 2021
@teor2345 teor2345 requested a review from dconnolly April 19, 2021 06:13
@teor2345 teor2345 self-assigned this Apr 19, 2021
@mpguerra mpguerra removed this from the 2021 Sprint 7 milestone Apr 19, 2021
@teor2345 teor2345 mentioned this pull request Apr 19, 2021
7 tasks
@teor2345 teor2345 changed the title Switch to an async mutex for handshake nonces (needs rebase after #2008 merges) Switch to an async mutex for handshake nonces Apr 19, 2021
@teor2345 teor2345 marked this pull request as ready for review April 19, 2021 23:11
Comment on lines +201 to +203
// It is ok to wait for the lock here, because handshakes have a short
// timeout, and the async mutex will be released when the task times
// out.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@dconnolly dconnolly merged commit 3f45735 into ZcashFoundation:main Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-bug Category: This is a bug C-cleanup Category: This is a cleanup C-security Category: Security issues I-hang A Zebra component stops responding to requests I-slow Problems with performance or responsiveness
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace sync::Mutex with a futures-aware Mutex
3 participants