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

BasicTokenBucket thread-safe issue #2328

Open
Lawliet828 opened this issue Oct 25, 2024 · 0 comments
Open

BasicTokenBucket thread-safe issue #2328

Lawliet828 opened this issue Oct 25, 2024 · 0 comments

Comments

@Lawliet828
Copy link

Lawliet828 commented Oct 25, 2024

  /**
   * Change rate and burst size.
   *
   * Warning: not thread safe!
   *
   * @param genRate Number of tokens to generate per second.
   * @param burstSize Maximum burst size. Must be greater than 0.
   * @param nowInSeconds Current time in seconds. Should be monotonically
   *                     increasing from the nowInSeconds specified in
   *                     this token bucket's constructor.
   */
  void reset(
      double genRate,
      double burstSize,
      double nowInSeconds = defaultClockNow()) noexcept {
    assert(genRate > 0);
    assert(burstSize > 0);
    const double availTokens = available(nowInSeconds);
    rate_ = genRate;
    burstSize_ = burstSize;
    setCapacity(availTokens, nowInSeconds);
  }

  /**
   * Attempts to consume some number of tokens. Tokens are first added to the
   * bucket based on the time elapsed since the last attempt to consume tokens.
   * Note: Attempts to consume more tokens than the burst size will always
   * fail.
   *
   * Thread-safe.
   *
   * @param toConsume The number of tokens to consume.
   * @param nowInSeconds Current time in seconds. Should be monotonically
   *                     increasing from the nowInSeconds specified in
   *                     this token bucket's constructor.
   * @return True if the rate limit check passed, false otherwise.
   */
  bool consume(double toConsume, double nowInSeconds = defaultClockNow()) {
    return tokenBucket_.consume(toConsume, rate_, burstSize_, nowInSeconds);
  }

if rate_ and burstSize_ are not atomic, the function consume, consumeOrDrain, available, balance, rate, burst of BasicTokenBucket are not thread-safe.

thread 1 : reset -> write rate_ and burstSize_
thread 2 : consume -> read rate_ and burstSize_

If there are two threads, one executing consume and the other executing reset simultaneously, it will cause the values of rate_ and burstSize_ to be indeterminate, potentially being neither the new nor the old values.

the reset interface is provided, as long as two threads access rate_ and burstSize_ simultaneously, and at least one of them is a modification operation, it will lead to thread safety issues.

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

No branches or pull requests

1 participant