Repository: llvm/llvm-project
File: clang/lib/DependencyScanning/InProcessModuleCache.cpp (C++)

Code before:
class ReaderWriterLock : public llvm::AdvisoryLock {
  ModuleCacheEntry &Entry;
  std::optional<unsigned> OwnedGeneration;

public:
  explicit ReaderWriterLock(ModuleCacheEntry &Entry) : Entry(Entry) {}

  Expected<bool> tryLock() override {
    std::lock_guard<std::mutex> Lock(Entry.Mutex);
    if (Entry.Locked)
      return false;
    Entry.Locked = true;
    OwnedGeneration = Entry.Generation;
    return true;
  }

  llvm::WaitForUnlockResult
  waitForUnlockFor(std::chrono::seconds MaxSeconds) override {
    assert(!OwnedGeneration);
    std::unique_lock<std::mutex> Lock(Entry.Mutex);
    unsigned CurrentGeneration = Entry.Generation;
    bool Success = Entry.CondVar.wait_for(Lock, MaxSeconds, [&] {
      // We check not only Locked, but also Generation to break the wait in case
      // of unsafeUnlock() and successful tryLock().
      return !Entry.Locked || Entry.Generation != CurrentGeneration;
    });
    return Success ? llvm::WaitForUnlockResult::Success
                   : llvm::WaitForUnlockResult::Timeout;
  }

  std::error_code unsafeUnlock() override {
    {
      std::lock_guard<std::mutex> Lock(Entry.Mutex);
      Entry.Generation += 1;
      Entry.Locked = false;
    }
    Entry.CondVar.notify_all();
    return {};
  }

  ~ReaderWriterLock() override {
    if (OwnedGeneration) {
      {
        std::lock_guard<std::mutex> Lock(Entry.Mutex);
        // Avoid stomping over the state managed by someone else after
        // unsafeUnlock() and successful tryLock().
        if (*OwnedGeneration == Entry.Generation)
          Entry.Locked = false;
      }
      Entry.CondVar.notify_all();
    }

Review comment (suggestion):
Note that this does now mean that readers will contend on `notify_all` because they need to acquire `Entry.Mutex` to return a result. Not sure how much this actually matters, and I don't immediately have a better solution.

Code after:
class ReaderWriterLock : public llvm::AdvisoryLock {
  ModuleCacheEntry &Entry;
  std::optional<unsigned> OwnedGeneration;

public:
  ReaderWriterLock(ModuleCacheEntry &Entry) : Entry(Entry) {}

  Expected<bool> tryLock() override {
    std::lock_guard<std::mutex> Lock(Entry.Mutex);
    if (Entry.Locked)
      return false;
    Entry.Locked = true;
    OwnedGeneration = Entry.Generation;
    return true;
  }

  llvm::WaitForUnlockResult
  waitForUnlockFor(std::chrono::seconds MaxSeconds) override {
    assert(!OwnedGeneration);
    std::unique_lock<std::mutex> Lock(Entry.Mutex);
    unsigned CurrentGeneration = Entry.Generation;
    bool Success = Entry.CondVar.wait_for(Lock, MaxSeconds, [&] {
      // We check not only Locked, but also Generation to break the wait in case
      // of unsafeUnlock() and successful tryLock().
      return !Entry.Locked || Entry.Generation != CurrentGeneration;
    });
    return Success ? llvm::WaitForUnlockResult::Success
                   : llvm::WaitForUnlockResult::Timeout;
  }

  std::error_code unsafeUnlock() override {
    {
      std::lock_guard<std::mutex> Lock(Entry.Mutex);
      Entry.Generation += 1;
      Entry.Locked = false;
    }
    Entry.CondVar.notify_all();
    return {};
  }

  ~ReaderWriterLock() override {
    if (OwnedGeneration) {
      {
        std::lock_guard<std::mutex> Lock(Entry.Mutex);
        // Avoid stomping over the state managed by someone else after
        // unsafeUnlock() and successful tryLock().
        if (*OwnedGeneration == Entry.Generation)
          Entry.Locked = false;
      }
      Entry.CondVar.notify_all();
    }