facebook/folly

BasicTokenBucket thread-safe issue

Opened this issue · 0 comments

  /**
   * 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.