Skip to content

LightweightSemaphore weak-try? #12

Open

Description

@Caldfir

Reading through the code here, I noted the following implementation of LightweightSemaphore::tryWait():

return (oldCount > 0 && m_count.compare_exchange_strong(oldCount, oldCount - 1, std::memory_order_acquire));

If I am not mistaken, it is possible for another thread to signal between the load and the compare_exchange_strong which would result in a spurious failure. I think this is commonly referred to as a "weak-try" but if that was the intent then using compare_exchange_weak would be sufficient. I only see this code being used in one single place in this project, and in that instance the strong/weak distinction wouldn't be relevant.

I'm certainly no expert on the topic of C++11 atomic operations, so I could be mistaken.

Activity

  1. changed the title [-]weak-try?[/-] [+]LightweightSemaphore weak-try?[/+] on Jul 5, 2021
  2. Caldfir commented on Jul 5, 2021

    @Caldfir
    Author

    To clarify, the issue is that it seems to be possible for tryWait to return false while the semaphore is positive because CAS could fail from a signal() in another thread. The only resolution I can think of would be to loop:

    int oldCount;
    while (1)
    {
      oldCount = m_count.load(std::memory_order_relaxed);
      if (oldCount <= 0) return false;
      if (m_count.compare_exchange_weak(oldCount, oldCount - 1, std::memory_order_acquire)) return true;
    }

    But while(1) in a method called try... sounds obscene, even if in even the worst case this would only actually loop very infrequently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions