Reader Writer Lock Implementation

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
1
down vote

favorite












I'm practicing to write c++11/14 and I implemented a RW lock. Does everything look ok?



class RWLock 
public:
void readLock()
std::unique_lock<std::mutex> lock(mutex);
waitingReaders.wait(lock, [this]!writer && queuedWriters == 0);
++readers;


void readUnlock()
std::unique_lock<std::mutex> lock(mutex);
--readers;
if (readers == 0 && queuedWriters > 0)
lock.unlock();
waitingWriters.notify_one();



void writeLock()
std::unique_lock<std::mutex> lock(mutex);
++queuedWriters;
waitingWriters.wait(lock, [this]!writer && readers == 0);
--queuedWriters;
writer = true;


void writeUnlock()
std::unique_lock<std::mutex> lock(mutex);

if(queuedWriters > 0)
lock.unlock();
waitingWriters.notify_one();
else
writer = false;
lock.unlock();
waitingReaders.notify_all();



private:
std::mutex mutex;
std::condition_variable waitingReaders;
std::condition_variable waitingWriters;

int readers = 0;
int queuedWriters = 0;
bool writer = false;
;


In particular:
Do I really need to take a lock in my WriteUnlock() method? It looks like all I'm doing is unlock right away.



And is it recommended to perform a lock.unlock() before I do a notify_one or a notify_all ?



Does performing a condition_variable.wait(lock, lambda) where the lambda is true cause a performance loss? Like potentially in the readLock() method







share|improve this question



























    up vote
    1
    down vote

    favorite












    I'm practicing to write c++11/14 and I implemented a RW lock. Does everything look ok?



    class RWLock 
    public:
    void readLock()
    std::unique_lock<std::mutex> lock(mutex);
    waitingReaders.wait(lock, [this]!writer && queuedWriters == 0);
    ++readers;


    void readUnlock()
    std::unique_lock<std::mutex> lock(mutex);
    --readers;
    if (readers == 0 && queuedWriters > 0)
    lock.unlock();
    waitingWriters.notify_one();



    void writeLock()
    std::unique_lock<std::mutex> lock(mutex);
    ++queuedWriters;
    waitingWriters.wait(lock, [this]!writer && readers == 0);
    --queuedWriters;
    writer = true;


    void writeUnlock()
    std::unique_lock<std::mutex> lock(mutex);

    if(queuedWriters > 0)
    lock.unlock();
    waitingWriters.notify_one();
    else
    writer = false;
    lock.unlock();
    waitingReaders.notify_all();



    private:
    std::mutex mutex;
    std::condition_variable waitingReaders;
    std::condition_variable waitingWriters;

    int readers = 0;
    int queuedWriters = 0;
    bool writer = false;
    ;


    In particular:
    Do I really need to take a lock in my WriteUnlock() method? It looks like all I'm doing is unlock right away.



    And is it recommended to perform a lock.unlock() before I do a notify_one or a notify_all ?



    Does performing a condition_variable.wait(lock, lambda) where the lambda is true cause a performance loss? Like potentially in the readLock() method







    share|improve this question























      up vote
      1
      down vote

      favorite









      up vote
      1
      down vote

      favorite











      I'm practicing to write c++11/14 and I implemented a RW lock. Does everything look ok?



      class RWLock 
      public:
      void readLock()
      std::unique_lock<std::mutex> lock(mutex);
      waitingReaders.wait(lock, [this]!writer && queuedWriters == 0);
      ++readers;


      void readUnlock()
      std::unique_lock<std::mutex> lock(mutex);
      --readers;
      if (readers == 0 && queuedWriters > 0)
      lock.unlock();
      waitingWriters.notify_one();



      void writeLock()
      std::unique_lock<std::mutex> lock(mutex);
      ++queuedWriters;
      waitingWriters.wait(lock, [this]!writer && readers == 0);
      --queuedWriters;
      writer = true;


      void writeUnlock()
      std::unique_lock<std::mutex> lock(mutex);

      if(queuedWriters > 0)
      lock.unlock();
      waitingWriters.notify_one();
      else
      writer = false;
      lock.unlock();
      waitingReaders.notify_all();



      private:
      std::mutex mutex;
      std::condition_variable waitingReaders;
      std::condition_variable waitingWriters;

      int readers = 0;
      int queuedWriters = 0;
      bool writer = false;
      ;


      In particular:
      Do I really need to take a lock in my WriteUnlock() method? It looks like all I'm doing is unlock right away.



      And is it recommended to perform a lock.unlock() before I do a notify_one or a notify_all ?



      Does performing a condition_variable.wait(lock, lambda) where the lambda is true cause a performance loss? Like potentially in the readLock() method







      share|improve this question













      I'm practicing to write c++11/14 and I implemented a RW lock. Does everything look ok?



      class RWLock 
      public:
      void readLock()
      std::unique_lock<std::mutex> lock(mutex);
      waitingReaders.wait(lock, [this]!writer && queuedWriters == 0);
      ++readers;


      void readUnlock()
      std::unique_lock<std::mutex> lock(mutex);
      --readers;
      if (readers == 0 && queuedWriters > 0)
      lock.unlock();
      waitingWriters.notify_one();



      void writeLock()
      std::unique_lock<std::mutex> lock(mutex);
      ++queuedWriters;
      waitingWriters.wait(lock, [this]!writer && readers == 0);
      --queuedWriters;
      writer = true;


      void writeUnlock()
      std::unique_lock<std::mutex> lock(mutex);

      if(queuedWriters > 0)
      lock.unlock();
      waitingWriters.notify_one();
      else
      writer = false;
      lock.unlock();
      waitingReaders.notify_all();



      private:
      std::mutex mutex;
      std::condition_variable waitingReaders;
      std::condition_variable waitingWriters;

      int readers = 0;
      int queuedWriters = 0;
      bool writer = false;
      ;


      In particular:
      Do I really need to take a lock in my WriteUnlock() method? It looks like all I'm doing is unlock right away.



      And is it recommended to perform a lock.unlock() before I do a notify_one or a notify_all ?



      Does performing a condition_variable.wait(lock, lambda) where the lambda is true cause a performance loss? Like potentially in the readLock() method









      share|improve this question












      share|improve this question




      share|improve this question








      edited May 24 at 1:37









      200_success

      123k14143399




      123k14143399









      asked May 24 at 1:06









      user3666471

      1293




      1293




















          1 Answer
          1






          active

          oldest

          votes

















          up vote
          2
          down vote














          Do I really need to take a lock in my WriteUnlock() method? It looks like all I'm doing is unlock right away.




          YES, you need the lock, because you are accessing queuedWriters and you need to make sure nobody changes it (by calling writeLock) before you notify readers or writers based on the check. You could have seen queuedWriters being zero, therefore notifying readers, but some writer could get sleeping in the meantime and never get woken.




          And is it recommended to perform a lock.unlock() before I do a notify_one or a notify_all?




          Unlock before, because the thread being woken would immediatelly try to hold the lock, thus being blocked again (to be woken for second time when you unlock).




          Does performing a condition_variable.wait(lock, lambda) where the lambda is true cause a performance loss?




          It is same as while (!pred()) wait(lck);
          What else could you do? It checks first, so, would not wait if the condition is true.






          share|improve this answer























          • I have also always wondered if you need to unlock() before a notify*(). But I have never found a reference with this recommendation (for or against). If you have a reference for this I would love to read it. Currently I am not convinced by your logic; just because you notify the conditional does not mean that anything happens immediately. Note: I am not saying it is bad advice (it seems logical and does not seem to break things) but I would love to read from an authoritative reference.
            – Martin York
            May 24 at 21:16











          • @MartinYork stackoverflow.com/questions/17101922/…
            – firda
            May 24 at 22:01










          Your Answer




          StackExchange.ifUsing("editor", function ()
          return StackExchange.using("mathjaxEditing", function ()
          StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
          StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
          );
          );
          , "mathjax-editing");

          StackExchange.ifUsing("editor", function ()
          StackExchange.using("externalEditor", function ()
          StackExchange.using("snippets", function ()
          StackExchange.snippets.init();
          );
          );
          , "code-snippets");

          StackExchange.ready(function()
          var channelOptions =
          tags: "".split(" "),
          id: "196"
          ;
          initTagRenderer("".split(" "), "".split(" "), channelOptions);

          StackExchange.using("externalEditor", function()
          // Have to fire editor after snippets, if snippets enabled
          if (StackExchange.settings.snippets.snippetsEnabled)
          StackExchange.using("snippets", function()
          createEditor();
          );

          else
          createEditor();

          );

          function createEditor()
          StackExchange.prepareEditor(
          heartbeatType: 'answer',
          convertImagesToLinks: false,
          noModals: false,
          showLowRepImageUploadWarning: true,
          reputationToPostImages: null,
          bindNavPrevention: true,
          postfix: "",
          onDemand: true,
          discardSelector: ".discard-answer"
          ,immediatelyShowMarkdownHelp:true
          );



          );








           

          draft saved


          draft discarded


















          StackExchange.ready(
          function ()
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f195055%2freader-writer-lock-implementation%23new-answer', 'question_page');

          );

          Post as a guest






























          1 Answer
          1






          active

          oldest

          votes








          1 Answer
          1






          active

          oldest

          votes









          active

          oldest

          votes






          active

          oldest

          votes








          up vote
          2
          down vote














          Do I really need to take a lock in my WriteUnlock() method? It looks like all I'm doing is unlock right away.




          YES, you need the lock, because you are accessing queuedWriters and you need to make sure nobody changes it (by calling writeLock) before you notify readers or writers based on the check. You could have seen queuedWriters being zero, therefore notifying readers, but some writer could get sleeping in the meantime and never get woken.




          And is it recommended to perform a lock.unlock() before I do a notify_one or a notify_all?




          Unlock before, because the thread being woken would immediatelly try to hold the lock, thus being blocked again (to be woken for second time when you unlock).




          Does performing a condition_variable.wait(lock, lambda) where the lambda is true cause a performance loss?




          It is same as while (!pred()) wait(lck);
          What else could you do? It checks first, so, would not wait if the condition is true.






          share|improve this answer























          • I have also always wondered if you need to unlock() before a notify*(). But I have never found a reference with this recommendation (for or against). If you have a reference for this I would love to read it. Currently I am not convinced by your logic; just because you notify the conditional does not mean that anything happens immediately. Note: I am not saying it is bad advice (it seems logical and does not seem to break things) but I would love to read from an authoritative reference.
            – Martin York
            May 24 at 21:16











          • @MartinYork stackoverflow.com/questions/17101922/…
            – firda
            May 24 at 22:01














          up vote
          2
          down vote














          Do I really need to take a lock in my WriteUnlock() method? It looks like all I'm doing is unlock right away.




          YES, you need the lock, because you are accessing queuedWriters and you need to make sure nobody changes it (by calling writeLock) before you notify readers or writers based on the check. You could have seen queuedWriters being zero, therefore notifying readers, but some writer could get sleeping in the meantime and never get woken.




          And is it recommended to perform a lock.unlock() before I do a notify_one or a notify_all?




          Unlock before, because the thread being woken would immediatelly try to hold the lock, thus being blocked again (to be woken for second time when you unlock).




          Does performing a condition_variable.wait(lock, lambda) where the lambda is true cause a performance loss?




          It is same as while (!pred()) wait(lck);
          What else could you do? It checks first, so, would not wait if the condition is true.






          share|improve this answer























          • I have also always wondered if you need to unlock() before a notify*(). But I have never found a reference with this recommendation (for or against). If you have a reference for this I would love to read it. Currently I am not convinced by your logic; just because you notify the conditional does not mean that anything happens immediately. Note: I am not saying it is bad advice (it seems logical and does not seem to break things) but I would love to read from an authoritative reference.
            – Martin York
            May 24 at 21:16











          • @MartinYork stackoverflow.com/questions/17101922/…
            – firda
            May 24 at 22:01












          up vote
          2
          down vote










          up vote
          2
          down vote










          Do I really need to take a lock in my WriteUnlock() method? It looks like all I'm doing is unlock right away.




          YES, you need the lock, because you are accessing queuedWriters and you need to make sure nobody changes it (by calling writeLock) before you notify readers or writers based on the check. You could have seen queuedWriters being zero, therefore notifying readers, but some writer could get sleeping in the meantime and never get woken.




          And is it recommended to perform a lock.unlock() before I do a notify_one or a notify_all?




          Unlock before, because the thread being woken would immediatelly try to hold the lock, thus being blocked again (to be woken for second time when you unlock).




          Does performing a condition_variable.wait(lock, lambda) where the lambda is true cause a performance loss?




          It is same as while (!pred()) wait(lck);
          What else could you do? It checks first, so, would not wait if the condition is true.






          share|improve this answer
















          Do I really need to take a lock in my WriteUnlock() method? It looks like all I'm doing is unlock right away.




          YES, you need the lock, because you are accessing queuedWriters and you need to make sure nobody changes it (by calling writeLock) before you notify readers or writers based on the check. You could have seen queuedWriters being zero, therefore notifying readers, but some writer could get sleeping in the meantime and never get woken.




          And is it recommended to perform a lock.unlock() before I do a notify_one or a notify_all?




          Unlock before, because the thread being woken would immediatelly try to hold the lock, thus being blocked again (to be woken for second time when you unlock).




          Does performing a condition_variable.wait(lock, lambda) where the lambda is true cause a performance loss?




          It is same as while (!pred()) wait(lck);
          What else could you do? It checks first, so, would not wait if the condition is true.







          share|improve this answer















          share|improve this answer



          share|improve this answer








          edited May 24 at 11:07









          yuri

          3,3872832




          3,3872832











          answered May 24 at 9:47









          firda

          2,184525




          2,184525











          • I have also always wondered if you need to unlock() before a notify*(). But I have never found a reference with this recommendation (for or against). If you have a reference for this I would love to read it. Currently I am not convinced by your logic; just because you notify the conditional does not mean that anything happens immediately. Note: I am not saying it is bad advice (it seems logical and does not seem to break things) but I would love to read from an authoritative reference.
            – Martin York
            May 24 at 21:16











          • @MartinYork stackoverflow.com/questions/17101922/…
            – firda
            May 24 at 22:01
















          • I have also always wondered if you need to unlock() before a notify*(). But I have never found a reference with this recommendation (for or against). If you have a reference for this I would love to read it. Currently I am not convinced by your logic; just because you notify the conditional does not mean that anything happens immediately. Note: I am not saying it is bad advice (it seems logical and does not seem to break things) but I would love to read from an authoritative reference.
            – Martin York
            May 24 at 21:16











          • @MartinYork stackoverflow.com/questions/17101922/…
            – firda
            May 24 at 22:01















          I have also always wondered if you need to unlock() before a notify*(). But I have never found a reference with this recommendation (for or against). If you have a reference for this I would love to read it. Currently I am not convinced by your logic; just because you notify the conditional does not mean that anything happens immediately. Note: I am not saying it is bad advice (it seems logical and does not seem to break things) but I would love to read from an authoritative reference.
          – Martin York
          May 24 at 21:16





          I have also always wondered if you need to unlock() before a notify*(). But I have never found a reference with this recommendation (for or against). If you have a reference for this I would love to read it. Currently I am not convinced by your logic; just because you notify the conditional does not mean that anything happens immediately. Note: I am not saying it is bad advice (it seems logical and does not seem to break things) but I would love to read from an authoritative reference.
          – Martin York
          May 24 at 21:16













          @MartinYork stackoverflow.com/questions/17101922/…
          – firda
          May 24 at 22:01




          @MartinYork stackoverflow.com/questions/17101922/…
          – firda
          May 24 at 22:01












           

          draft saved


          draft discarded


























           


          draft saved


          draft discarded














          StackExchange.ready(
          function ()
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f195055%2freader-writer-lock-implementation%23new-answer', 'question_page');

          );

          Post as a guest













































































          Popular posts from this blog

          Greedy Best First Search implementation in Rust

          Function to Return a JSON Like Objects Using VBA Collections and Arrays

          C++11 CLH Lock Implementation