Tokio: Confusing Debug for tokio semaphore

[0xdeafbeef]

Version
0.2.23

Platform
All
Description
Debug for tokio::sync::Semaphore is very confusing.
It shows permits<<1, which is unexpected and can cause hours of debugging.

use tokio::sync::Semaphore;

fn main() {
    let a = 500;
    let sem = Semaphore::new(a);
    dbg!(a);
    dbg!(&sem);
}

[src/main.rs:8] a = 500
[src/main.rs:9] &sem = Semaphore {
    ll_sem: Semaphore {
        permits: 1000,
    },
}

[blasrodri]

This occurs because the permits number passed to Semaphore::new is left shifted by PERMIT_SHIFT.
I didn't see anything in the docs explaining why we need to shift that original value. So I'm not sure whether there's docs missing, or what's the best way to go about this.

References:
https://github.com/tokio-rs/tokio/blob/b44ab273597d3757b5b50eed95c4f8890fa54e42/tokio/src/sync/batch_semaphore.rs#L103

https://github.com/tokio-rs/tokio/blob/b44ab273597d3757b5b50eed95c4f8890fa54e42/tokio/src/sync/batch_semaphore.rs#L113

[hawkw]

This is because the least-significant bit in the number of permits is reserved to use as a flag indicating that the semaphore has been closed:
https://github.com/tokio-rs/tokio/blob/1167c09ae8bfd0d4fff97d33803d959222f9e2ab/tokio/src/sync/batch_semaphore.rs#L102

I think the right solution is to modify the fmt::Debug implementation for batch_semaphore::Semaphore:
https://github.com/tokio-rs/tokio/blob/1167c09ae8bfd0d4fff97d33803d959222f9e2ab/tokio/src/sync/batch_semaphore.rs#L357-L363

We could either change it to shift right by PERMIT_SHIFT, like this:

impl fmt::Debug for Semaphore {
    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
        fmt.debug_struct("Semaphore")
            .field("permits", &self.permits.load(Relaxed) >> Self::PERMIT_SHIFT)
            .finish()
    }
}

or just change it to call self.available_permits(), which would shift to the correct value internally:

impl fmt::Debug for Semaphore {
    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
        fmt.debug_struct("Semaphore")
            .field("permits", &self.available_permits())
            .finish()
    }
}

The second approach might be a little nicer, but they're equivalent. I would happily approve a PR that uses either solution.

Sorry for any confusion!