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.

Sign up for GitHub

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

Jump to bottom

Semaphore example using an Arc<Semaphore> for semaphore passing #5956

Merged
merged 33 commits into from Sep 19, 2023
Merged

Semaphore example using an Arc<Semaphore> for semaphore passing #5956

merged 33 commits into from Sep 19, 2023

Conversation

alexanderkirilin
Copy link
Contributor

A simple example of limiting request processing using a task-safe semaphore and demonstrating acquire_owned for: #5933

@github-actions github-actions bot added the R-loom-sync Run loom sync tests on this PR label Aug 27, 2023
@alexanderkirilin alexanderkirilin mentioned this pull request Aug 27, 2023
Closed
@Darksonn Darksonn added T-docs Topic: documentation A-tokio Area: The main tokio crate M-sync Module: tokio/sync labels Aug 28, 2023
@Darksonn
Copy link
Contributor

Your branch is behind #5939, which has already been merged. Would you be able to update your branch to latest master? You should be able to do this by merging master into your branch and fixing the resulting conflicts.

Darksonn
Darksonn reviewed Aug 29, 2023
View reviewed changes
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
@alexanderkirilin
Copy link
Contributor Author

alexanderkirilin commented Aug 29, 2023 •
edited

@Darksonn I think the event handler loop in the updated example is causing issues with CI.

The test suite has been running for 30 minutes so far, and I can't find a way to cancel the workflows.

Would you be able to stop it? Workflow: https://github.com/tokio-rs/tokio/actions/runs/6014142339

Darksonn
Darksonn reviewed Aug 29, 2023
View reviewed changes
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

I think this is looking pretty good. The example could use a few more comments on the semaphore-related stuff.

tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
alexanderkirilin and others added 4 commits August 29, 2023 12:49
@alexanderkirilin
Add comment
b17bddc
@alexanderkirilin @Darksonn
Update tokio/src/sync/semaphore.rs
1a7a25b 
Co-authored-by: Alice Ryhl <aliceryhl@google.com>
@alexanderkirilin
Merge remote-tracking branch 'origin/semaphore-example-file' into sem…
a60ff46 
…aphore-example-file

# Conflicts:
#	tokio/src/sync/semaphore.rs
@alexanderkirilin
Update
bd20813
Darksonn
Darksonn reviewed Aug 31, 2023
View reviewed changes
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
alexanderkirilin and others added 4 commits August 31, 2023 07:16
@alexanderkirilin @Darksonn
Update tokio/src/sync/semaphore.rs
ae49b96 
Co-authored-by: Alice Ryhl <aliceryhl@google.com>
@alexanderkirilin
Update
77d465b
@alexanderkirilin
Update
8de8bef
@alexanderkirilin
Update
8fd7a16
Darksonn
Darksonn reviewed Sep 1, 2023
View reviewed changes
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

It's looking good. I have a few comments to simplify the language a bit.

tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
alexanderkirilin and others added 6 commits September 1, 2023 08:54
@alexanderkirilin @Darksonn
Update tokio/src/sync/semaphore.rs
b92d933 
Co-authored-by: Alice Ryhl <aliceryhl@google.com>
@alexanderkirilin @Darksonn
Update tokio/src/sync/semaphore.rs
cf4f6fb 
Co-authored-by: Alice Ryhl <aliceryhl@google.com>
@alexanderkirilin @Darksonn
Update tokio/src/sync/semaphore.rs
87af825 
Co-authored-by: Alice Ryhl <aliceryhl@google.com>
@alexanderkirilin @Darksonn
Update tokio/src/sync/semaphore.rs
05a2900 
Co-authored-by: Alice Ryhl <aliceryhl@google.com>
@alexanderkirilin @Darksonn
Update tokio/src/sync/semaphore.rs
f8afd1f 
Co-authored-by: Alice Ryhl <aliceryhl@google.com>
@alexanderkirilin
Update
b4790c4
@alexanderkirilin
Copy link
Contributor Author

@Darksonn Thank you for the suggestions. This looks much more concise and focused than what I had begun with; and I've learned a few things about technical writing by reflecting on them.

Darksonn
Darksonn reviewed Sep 3, 2023
View reviewed changes
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

I think these suggestions are the last ones. Then I'm happy to merge this.

tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved
tokio/src/sync/semaphore.rs Show resolved Hide resolved
alexanderkirilin and others added 5 commits September 6, 2023 07:31
@alexanderkirilin @Darksonn
Update tokio/src/sync/semaphore.rs
b364ca8 
Co-authored-by: Alice Ryhl <aliceryhl@google.com>
@alexanderkirilin @Darksonn
Update tokio/src/sync/semaphore.rs
9180840 
Co-authored-by: Alice Ryhl <aliceryhl@google.com>
@alexanderkirilin @Darksonn
Update tokio/src/sync/semaphore.rs
e18f86c 
Co-authored-by: Alice Ryhl <aliceryhl@google.com>
@alexanderkirilin
Merge branch 'master' into semaphore-example-file
7a94754
@alexanderkirilin
Update
1158412
Darksonn
Darksonn approved these changes Sep 6, 2023
View reviewed changes
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me.

@alexanderkirilin
Copy link
Contributor Author

@Darksonn Merged latest commits from upstream. Would this be able to be merged?

@Darksonn Darksonn merged commit e6553c4 into tokio-rs:master Sep 19, 2023
70 checks passed
@Darksonn
Copy link
Contributor

Sorry, seems like I forgot to press the button.

@satakuma satakuma mentioned this pull request Oct 8, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Darksonn Darksonn Darksonn approved these changes

Assignees
No one assigned
Labels
A-tokio Area: The main tokio crate M-sync Module: tokio/sync R-loom-sync Run loom sync tests on this PR T-docs Topic: documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@alexanderkirilin @Darksonn