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

feat(gstd): Introduce max ownership time for Mutex #2912

Merged
merged 20 commits into from
Sep 4, 2023

Conversation

DennisInSky
Copy link
Member

This is an idea of supporting max ownership time for Mutex (the same will be implemented for RwLock if everyone happy with this approach). Basically, before acquiring the mutex ownership, one can declare in the code max number of blocks they are going to own the mutex for. If there happens another attempt to grab the same mutex after the specified number of blocks, but the mutex has not been released by the first owner yet, the first message execution will get terminated via panic indicating the message exceeded lock ownership time.

@reviewer-or-team

@DennisInSky DennisInSky added the A0-pleasereview PR is ready to be reviewed by the team label Jul 6, 2023
@DennisInSky DennisInSky self-assigned this Jul 6, 2023
gstd/src/lock/mutex.rs Outdated Show resolved Hide resolved
@DennisInSky DennisInSky removed the A0-pleasereview PR is ready to be reviewed by the team label Jul 13, 2023
@DennisInSky DennisInSky added the A0-pleasereview PR is ready to be reviewed by the team label Jul 14, 2023
pallets/gear/src/tests.rs Show resolved Hide resolved
gstd/src/lock/mutex.rs Outdated Show resolved Hide resolved
gstd/src/lock/mutex.rs Outdated Show resolved Hide resolved
gstd/src/lock/mutex.rs Outdated Show resolved Hide resolved
gstd/src/lock/mutex.rs Outdated Show resolved Hide resolved
gstd/src/lock/mutex.rs Outdated Show resolved Hide resolved
@breathx breathx added A3-gotissues PR occurred to have issues after the review and removed A0-pleasereview PR is ready to be reviewed by the team labels Aug 6, 2023
@DennisInSky DennisInSky added A0-pleasereview PR is ready to be reviewed by the team and removed A3-gotissues PR occurred to have issues after the review labels Aug 21, 2023
@NikVolf
Copy link
Member

NikVolf commented Aug 24, 2023

Re-review @breathx ?

gstd/src/lock/mutex.rs Outdated Show resolved Hide resolved
Copy link
Member

@clearloop clearloop left a comment

Choose a reason for hiding this comment

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

I'm considering about providing a handbook for program developers since we are introducing more and more features now, the Learn Gear "std" library redirects to https://docs.gear.rs/gstd/ (plus https://wiki.gear-tech.io/docs/getting-started-in-5-minutes/) are obvious not friendly enough for new developers

examples/waiter/src/lib.rs Outdated Show resolved Hide resolved
gstd/src/lock/mutex.rs Outdated Show resolved Hide resolved
gstd/src/lock/mutex.rs Outdated Show resolved Hide resolved
gstd/src/lock/mutex.rs Outdated Show resolved Hide resolved
gstd/src/lock/mutex.rs Outdated Show resolved Hide resolved
gstd/src/lock/mutex.rs Outdated Show resolved Hide resolved
gstd/src/config.rs Show resolved Hide resolved
gstd/src/lock/mutex.rs Outdated Show resolved Hide resolved
@DennisInSky
Copy link
Member Author

@clearloop please re-review

Copy link
Member

@clearloop clearloop left a comment

Choose a reason for hiding this comment

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

lgtm!

@DennisInSky DennisInSky removed the A0-pleasereview PR is ready to be reviewed by the team label Sep 4, 2023
@DennisInSky DennisInSky merged commit ec462c3 into master Sep 4, 2023
8 checks passed
@DennisInSky DennisInSky deleted the dd/mx-own-up-for branch September 4, 2023 08:22
@shamilsan shamilsan added the B1-releasenotes The feature deserves to be added to the Release Notes label Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B1-releasenotes The feature deserves to be added to the Release Notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants