title: Locking SAL with Visual Studio
body: <p>I'm trying to add locking SAL statements to find/prevent incorrect locking in my application. Like missing locking calls and mismatched locking calls. I'm getting warnings I don't understand. I've put them as comments in the example below. I turned on code analysis and set Microsoft All Rules. I'm using Visual Studio 2019 16.11.9. I've also tried Visual Studio 2022 17.0.5 and get the same results.</p>
  <p>I've tried _Success_ in place of _When_. I've tried returning bool instead of BOOL and _Success_(return) vs _Success_(return != 0).</p>
  <p>Some research I've read:</p>
  <p><a href="https://docs.microsoft.com/en-us/cpp/code-quality/c26135?view=msvc-170" rel="nofollow noreferrer">https://docs.microsoft.com/en-us/cpp/code-quality/c26135?view=msvc-170</a></p>
  <pre><code>_When_(return != 0, _Acquires_lock_(p-&gt;cs))
  int TryEnter(DATA* p)
  {
      if (p-&gt;state != 0)
      {
          EnterCriticalSection(&amp;p-&gt;cs);
          return p-&gt;state;
      }
      return 0;
  }
  </code></pre>
  <p>The former article is from 2021 so is probably more up to date than the later one which is for Visual Studio 2015. The VS2015 example doesn't work as is; it would seem the _Success_ annotation has been changed as it no longer takes two parameters.</p>
  <p><a href="https://docs.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2015/code-quality/best-practices-and-examples-sal?view=vs-2015" rel="nofollow noreferrer">https://docs.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2015/code-quality/best-practices-and-examples-sal?view=vs-2015</a></p>
  <pre><code>// Incorrect  
  _Success_(return == TRUE, _Acquires_lock_(*lpCriticalSection))  
  BOOL WINAPI TryEnterCriticalSection(  
    _Inout_ LPCRITICAL_SECTION lpCriticalSection  
  );  
    
  // Correct  
  _Success_(return != 0, _Acquires_lock_(*lpCriticalSection))  
  BOOL WINAPI TryEnterCriticalSection(  
    _Inout_ LPCRITICAL_SECTION lpCriticalSection  
  );  
  </code></pre>
  <p>My test code:</p>
  <pre><code>#define _WIN32_WINNT _WIN32_WINNT_WIN7
  
  #include &lt;windows.h&gt;
  #include &lt;tchar.h&gt;
  
  // https://docs.microsoft.com/en-us/cpp/code-quality/c26135?view=msvc-170
  
  class CLock {
  public:
  
      CLock() noexcept : m_hMutex(CreateMutex(nullptr,FALSE,nullptr)){
      }
  
      ~CLock(){
  
          CloseHandle(m_hMutex);
  
          m_hMutex = nullptr;
  
      }
  
      _When_(return != 0, _Acquires_lock_(this-&gt;m_hMutex)) BOOL Lock() noexcept {
  
          // warning C26135: Missing annotation _Acquires_lock_(this-&gt;m_hMutex) at function 'CLock::Lock'.
          // I don't understand this warning, I have the annotation?
          return (WaitForSingleObject(m_hMutex,INFINITE) == WAIT_OBJECT_0);
  
      }
  
      _When_(return != 0, _Releases_lock_(this-&gt;m_hMutex)) BOOL Unlock() noexcept {
  
          // warning C26135 : Missing annotation _Releases_lock_(this-&gt;m_hMutex) at function 'CLock::Unlock'.
          // I don't understand this warning, I have the annotation?
          return ReleaseMutex(m_hMutex);
  
      }
  
  private:
  
      // Ensure no copies can be made
      CLock(const CLock &amp;lCopy) = delete;
      CLock(const CLock &amp;&amp;lMove) = delete;
      const CLock &amp;operator=(const CLock &amp;lCopy) = delete;
      const CLock &amp;&amp;operator=(const CLock &amp;&amp;lMove) = delete;
  
      _Has_lock_kind_(_Lock_kind_mutex_) HANDLE m_hMutex;
  
  };
  
  class ThreadedThingy {
  public:
  
      ThreadedThingy() noexcept : m_iThingy(0){
      }
  
      VOID TestBadNotLocked() noexcept {
  
          // warning C26130: Missing annotation _Requires_lock_held_(this-&gt;m_lLock) or _No_competing_thread_
          //                 at function 'ThreadedThingy::TestBadNotLocked'. Otherwise it could be a race
          //                 condition. Variable 'this-&gt;m_iThingy' should be protected by lock 'this-&gt;m_lLock'.
          // This should be a warning since the lock isn't held
          m_iThingy = 10;
  
      }
  
      VOID TestBadNotUnlocked() noexcept {
  
          if (m_lLock.Lock()){
  
              // warning C26165: Possibly failing to release lock '(&amp;this-&gt;m_lLock)-&gt;m_hMutex' in function
              //                'ThreadedThingy::TestBadNotUnlocked'.
              // This should be a warning since lock isn't unlocked
  
              // warning C26130: Missing annotation _Requires_lock_held_(this-&gt;m_lLock) or _No_competing_thread_
              //                 at function 'ThreadedThingy::TestBadNotUnlocked'. Otherwise it could be a race
              //                 condition. Variable 'this-&gt;m_iThingy' should be protected by lock 'this-&gt;m_lLock'.
              // I don't understand this warning, the lock is held
              m_iThingy = 20;
  
          }
  
      }
  
      VOID TestGood() noexcept {
  
          if (m_lLock.Lock()){
  
              // warning C26130: Missing annotation _Requires_lock_held_(this-&gt;m_lLock) or _No_competing_thread_ at
              //                 function 'ThreadedThingy::TestGood'. Otherwise it could be a race condition.
              //                 Variable 'this-&gt;m_iThingy' should be protected by lock 'this-&gt;m_lLock'.
              // I don't understand this warning, the lock is held
              m_iThingy = 20;
  
              // warning C26165: Possibly failing to release lock '(&amp;this-&gt;m_lLock)-&gt;m_hMutex' in function
              //                 'ThreadedThingy::TestGood'.
              // I kind of understand this warning because Unlock can't gaurentee the lock is released
              m_lLock.Unlock();
  
          }
  
      }
  
  private:
  
      _Has_lock_kind_(_Lock_kind_mutex_) CLock m_lLock;
  
      _Guarded_by_(this-&gt;m_lLock) INT m_iThingy;
  
  };
  
  INT _tmain() noexcept {
  
      ThreadedThingy ttTest;
  
      ttTest.TestBadNotLocked();
  
      ttTest.TestBadNotUnlocked();
  
      ttTest.TestGood();
  
      return 0;
  
  }
  </code></pre>
  <p>Thanks!</p>
  <p>-Daniel</p>