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

Support a shared Semaphor #9

Closed
jean-airoldie opened this issue Oct 8, 2019 · 5 comments
Closed

Support a shared Semaphor #9

jean-airoldie opened this issue Oct 8, 2019 · 5 comments

Comments

@jean-airoldie
Copy link
Contributor

A Semaphore with a SemaphoreReleaser without a lifetime parameter would be quite useful. I imagine that using a design similar to channel::shared would do the trick.

@Matthias247
Copy link
Owner

The reason this is not included is that I actually imagined a composition through Arc<Semaphore> being sufficient. Channels have dedicated shared types to be able to split senders and receivers and to be able to hide them easier behind traits. But for Mutex/Semaphore/Event I didn’t see a need for it.

What would be the use-case where the external Arc isn’t sufficient?

@jean-airoldie
Copy link
Contributor Author

jean-airoldie commented Oct 9, 2019

Say you had:

struct Foo {
  sema: Arc<Semaphore>,
}

impl Foo {
  async fn bar(&self) -> Bar {
    Bar {
      permits: self.sema.acquire(1).await,
    }
  }
}

struct Bar {
  permits: SemaphoreReleaser<???>,
}

What would be the lifetime of the SemaphoreReleaser? If its static, then bar borrows Foo for static. If its not static then Bar cannot be used on a separate thread.

You could put a copy of the semaphore in Bar and make Bar a self referencing struct, but that seems overly complicated.

@Matthias247
Copy link
Owner

Matthias247 commented Oct 9, 2019

It would have a lifetime of 'self. Or actually it would be a little bit more messy, due to how lifetime interference in async fns works. I agree that if you want to store the releaser somewhere else, it isn't ideal. However it's not the typical use-case of the semaphore. That one is more like:

// Acquire one or more permits, e.g. to limit resources
let _releaser = self.sema.acquire(1).await;

do_another_async_action_which_needs_to_be_guarded().await;

drop(_releaser); // or implicit drop

The store permits and release elsewhere use-case is more rare. But nevertheless it exists - that's why the explicit release and disarm methods exist. Would they be applicable for your use-case in combination with Arc<Semaphore>? You use them together with a custom releaser type to keep the RAII semantics.

struct ArcReleaser {
    sem: Arc<Semaphore>,
    permits: usize,
}

impl ArcReleaser {
    fn from_releaser(releaser: SemaphoreReleaser, sem: Arc<Semaphore>) -> ArcReleaser {
        let permits = releaser.disarm(); // Take over commits
        ArcReleaser {
            sem,
            permits,
    }
}

impl Drop for ArcReleaser {
    fn drop(&mut self) {
        self.sem.release(self.permits);
    }
}

async fn your_async_fn() {
    let releaser = arc_sem.acquire(1).await;
    let arc_releaser = ArcReleaser::from_releaser(releaser, arc_sem.clone());
    // Can now move around arc_releaser without lifetime
}

I am aware that's more boilerplate than a builtin shared type. However as you discovered the code-duplication for adding those types isn't ideal, and I would prefer to keep the library surface as small and maintainable as possible.

@jean-airoldie
Copy link
Contributor Author

It would have a lifetime of 'self. Or actually it would be a little bit more messy, due to how lifetime interference in async fns works.

Yep, but as far as I know you can't enforce a 'self lifetime.

Would they be applicable for your use-case in combination with Arc?

Sure that would work. I agree that the code-duplication is quite horrendous.

My use case is to implement a bounded channel where each element has arbitrary weight (in bytes). Its pretty usefull when doing I/O. I don't know if that's considered a rare use-case.

I guess I'll write a small wrapper based on you suggestion.

@Matthias247
Copy link
Owner

Matthias247 commented Oct 11, 2019

My use case is to implement a bounded channel where each element has arbitrary weight (in bytes). Its pretty usefull when doing I/O. I don't know if that's considered a rare use-case.

That's actually a great use for an async semaphore! I required something similar recently, where I wanted to give various streams a flow-control budget. I however had use-cases where I could not use Arcs for that purpose (due to pure no-std) and therefore stored the permits as plain number.

I guess I'll write a small wrapper based on you suggestion.

Sounds good. I will close your PR then.
Maybe it makes sense to include the code from above as

SemaphoreReleaser::to_owned(self, sem: Arc<Semaphore>) -> ArcSemaphoreReleaser

which is like a compromise on between duplicating it in every application and adding a full shared semaphore with all the duplication to the library. But I think I want to wait for a bit more of experiences/application usages on this. Adding the few lines to projects which need it isn't too bad to make it work.

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 a pull request may close this issue.

2 participants