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

Add RwLock to mc-sgx-tstdc #260

Merged
merged 2 commits into from
Jan 27, 2023
Merged

Add RwLock to mc-sgx-tstdc #260

merged 2 commits into from
Jan 27, 2023

Conversation

nick-mobilecoin
Copy link
Collaborator

No description provided.

@github-actions github-actions bot requested review from jcape and varsha888 January 25, 2023 04:27
@github-actions github-actions bot added the size/L PRs with more than 500 lines of changes label Jan 25, 2023
This was referenced Jan 25, 2023
@nick-mobilecoin
Copy link
Collaborator Author

nick-mobilecoin commented Jan 25, 2023

@codecov-commenter
Copy link

codecov-commenter commented Jan 25, 2023

Codecov Report

Merging #260 (599094d) into main (655298e) will not change coverage.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main     #260   +/-   ##
=======================================
  Coverage   90.30%   90.30%           
=======================================
  Files          45       45           
  Lines        3949     3949           
=======================================
  Hits         3566     3566           
  Misses        383      383           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@jcape jcape left a comment

Choose a reason for hiding this comment

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

LGTM, but with the same suggestion as on Mutex: recommend Result<bool, Error> for try_lock()-esque calls that can simply not grab the lock vs. fail for other reasons.

Base automatically changed from nick/condvar to main January 25, 2023 22:14
Previously `try_lock()` would return `Ok(())` or `Err(Error)`. This
resulted in `Error::Busy` coming back in `Err()` when another thread had
the lock. Another thread owning the lock in a `try_lock()` call is
expected behavior and isn't a traditional _error_. Now `try_lock()`
returns a `bool` in normal expected operation to indicate if the lock
was acquired or not.
@nick-mobilecoin
Copy link
Collaborator Author

nick-mobilecoin commented Jan 27, 2023

✅ This pull request merged successfully as part of a Graphite job
Stack job ID: bQcf6PkCMMIqJ14ZDqqq.
See details on graphite.dev

@nick-mobilecoin nick-mobilecoin merged commit f3a1d6b into main Jan 27, 2023
@nick-mobilecoin nick-mobilecoin deleted the nick/rwlock branch January 27, 2023 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L PRs with more than 500 lines of changes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants