SemaphorePermit::merge can overflow and lose permits

[Rachit2323]

Version
tokio master (latest)

Platform
Any. Tested on the Rust Playground.

Description

SemaphorePermit::merge just adds the two permit counts together with a plain +:

self.permits += other.permits;

permits is a u32, so if the two counts add up to more than ~4.29 billion, it overflows. In debug mode it panics ("attempt to add with overflow"); in release mode it silently wraps to a small wrong number, so when the permit is dropped it gives back way fewer permits than were taken — the semaphore quietly loses permits.

A semaphore can legitimately hold billions of permits (MAX_PERMITS is huge), so this is reachable.

Here's the code I ran (panics on the Playground):

use std::sync::Arc;
use tokio::sync::Semaphore;

fn main() {
    let sem = Arc::new(Semaphore::new(6_000_000_000));
    let mut a = sem.try_acquire_many(3_000_000_000).unwrap();
    let b = sem.try_acquire_many(3_000_000_000).unwrap();
    a.merge(b); // 3e9 + 3e9 = 6e9 > u32::MAX -> overflow
}

OwnedSemaphorePermit::merge has the same issue.

Proposed fix

Use checked_add so it fails clearly instead of silently wrapping:

self.permits = self.permits.checked_add(other.permits).expect("permit count overflow");
Happy to send a PR.

[Darksonn]

Perhaps SemaphorePermit should just use usize for the number of permits.