How to address the "Lock was destructed while held" Visual C++ runtime assertion

Question

In a program that uses DirectX to render a 3D component I use a dedicated rendering thread that apparently gets created if I call DispatcherQueueController::CreateOnDedicatedThread(). What needs to be rendered is influenced by actions done on the main thread which is why I use a scoped_lock to synchronize the access.

The code that creates the rendering thread and the code that runs within it looks as follows:

void MyCustomView::StartRenderingLoop()
{
  if(!_isRenderLoopRunning)
  {
    _renderLoopController = DispatcherQueueController::CreateOnDedicatedThread();
    _isRenderLoopRunning  = _renderLoopController.DispatcherQueue().TryEnqueue([this]()
    {
      while(_isRenderLoopRunning)
      {
        Concurrency::critical_section::scoped_lock lock(_criticalSection);

        if(_renderer->Render())
        {
          Present();
        }

        // Halt the thread until the next vblank is reached.
        // This ensures the app isn't updating and rendering faster than the display can refresh,
        // which would unnecessarily spend extra CPU and GPU resources. This helps improve battery life.
        _dxgiOutput->WaitForVBlank();
      }
    });
  }
}

The associated member variables in the C++ header file looks as follows:

private:
  concurrency::critical_section _criticalSection;
  winrt::Microsoft::UI::Dispatching::DispatcherQueueController _renderLoopController{ nullptr };
  bool _isRenderLoopRunning  = false;

In order to stop the rendering thread the destructor of the 3D component contains the following code:

MyCustomView::~MyCustomView()
{
  _isRenderLoopRunning = false;
  _renderLoopController.ShutdownQueueAsync();
}

When the 3D component gets destroyed, the Visual C++ runtime throws an assertion that looks as follows:

Debug Assertion Failed!

Program: MyAppPackage\CONCRT140D.dll
File: d:\agent\_work\18\s\src\vctools\crt\crtw32\concrt\rtlocks.cpp
Line: 1001

Expression: Lock was destructed while held

The callstack that I can obtain looks as follows:

concrt140d.dll!Concurrency::critical_section::~critical_section() + 79 bytes    Unknown
MyCustomComponents.dll!winrt::MyCustomComponents::implementation::BaseView::~BaseView() C++
MyCustomComponents.dll!winrt::implements<winrt::MyCustomComponents::implementation::MyCustomView,winrt::MyCustomComponents::MyCustomView,winrt::MyCustomComponents::implementation::BaseView,winrt::no_module_lock>::~implements<winrt::MyCustomComponents::implementation::MyCustomView,winrt::MyCustomComponents::MyCustomView,winrt::MyCustomComponents::implementation::BaseView,winrt::no_module_lock>()   C++
MyCustomComponents.dll!winrt::MyCustomComponents::implementation::MyCustomView_base<winrt::MyCustomComponents::implementation::MyCustomView,winrt::MyCustomComponents::implementation::BaseView>::~MyCustomView_base<winrt::MyCustomComponents::implementation::MyCustomView,winrt::MyCustomComponents::implementation::BaseView>() C++
MyCustomComponents.dll!winrt::MyCustomComponents::implementation::MyCustomView::~MyCustomView() Line 32 C++

I'm struggling to understand whether the assertion Lock was destructed while held points to an actual problem or whether it is safe to ignore this. If that is an actual problem, how would I address it properly?

Edit:

The call stack points to BaseView from which MyCustomView inherits. The simplified BaseView looks as follows:

#pragma once

#include <concrt.h>


namespace winrt::MyNamespace
{
  struct BaseView : BaseViewT<BaseView>
  {
  protected:
    concurrency::critical_section _criticalSection;

    // Several methods
  };
}

There is no constructor or destructor implementation of that class. But it constructs the _criticalSection as shown above.

If you use a debugger to catch the assertion, where in *your* code does it happen? — Some programmer dude, Jan 17 '22 at 08:46
`MyCustomView::~MyCustomView()` -- What happens if `MyCustomView` is copied or assigned anywhere? If you write a user-defined destructor, more than likely you need a user-defined copy constructor and assignment operator. — PaulMcKenzie, Jan 17 '22 at 08:49
@Someprogrammerdude If I break into my own code the debugger stops at the closing brace of the destructor `MyCustomView::~MyCustomView()`. — ackh, Jan 17 '22 at 08:53
@ackh You should be looking at the call stack, not just the place where the debugger stops. — PaulMcKenzie, Jan 17 '22 at 08:54
To emphasize the point of @PaulMcKenzie and the destructor, see [the rules of three, five and zero](https://en.cppreference.com/w/cpp/language/rule_of_three). — Some programmer dude, Jan 17 '22 at 08:56
The problem I'm facing is that I do not create instances of that class anywhere in my code. Instead, Microsoft's WinUI 3.0 framework creates those instances according to the XAML code. My XAML code defines one object shall be created. However, I don't know what the framework does exactly with this instruction. — ackh, Jan 17 '22 at 09:04
`if(!_isRenderLoopRunning)` -- Maybe not an issue, but I cringe when I see mere, simple booleans used to control how a multithread program operates, instead of using atomics, condition variables, etc. In addition, if the `StartRenderingLoop` can be called with different threads, it starts out without any synchronization. — PaulMcKenzie, Jan 17 '22 at 09:05
You should probably pass a weak reference into `TryEnqueue`, i.e. the return value of `get_weak()` in place of `this`. Inside the closure you would then add an outer `while`-loop that attempts to upgrade to a strong reference. See [Strong and weak references in C++/WinRT](https://learn.microsoft.com/en-us/windows/uwp/cpp-and-winrt-apis/weak-references) for details. — IInspectable, Jan 17 '22 at 13:23
You destruct the object while the render loop is still running, and you don't wait for th render loop to exit, so the render loop finds itself using already-destructed objects. — Raymond Chen, Mar 23 '22 at 01:30
There are many possible solutions. One is for the destructor to wait for the render loop to exit. Another is for the render loop to share a state object with the MyCustomView object. — Raymond Chen, Mar 24 '22 at 15:38
@ackh I agree with Raymond's analysis that `MyCustomView` and thus its member `_criticalSection` is destroyed while simultaneously the render thread is still running and is holding the lock. It takes a while until the `WaitForVBlank()` wakes up in the thread and until `_isRenderLoopRunning==false` is seen. Try [`_renderLoopController.ShutdownQueueAsync().get();`](https://learn.microsoft.com/en-us/windows/uwp/cpp-and-winrt-apis/concurrency#block-the-calling-thread) in the destructor of `MyCustomView` to wait until the render thread finished. That should fix the problem. — Sedenion, Apr 13 '22 at 20:05
@Sedenion Thanks for the suggestion. I do agree that this is likely a problem. However, I tried both `_renderLoopController.ShutdownQueueAsync().GetResults()` and `_renderLoopController.ShutdownQueueAsync().get()` so far and the same problem still remains, i.e. the assertion is still firing. — ackh, Apr 14 '22 at 06:39
According to the call stack, the problematic critical section ist destructed from `BaseView::~BaseView()`. This suggests that `BaseView` has also a critical section as member. Can you show the class definition of `BaseView` and of it's destructor, and also where this other critical section is locked? — Sedenion, Apr 14 '22 at 15:35
@Sedenion Right, the call stack lists the destructor of `BaseView`. However, there isn't any implementation of such a destructor. But `_criticalSection` is part of that class. So, `BaseView` will eventually have to destroy that object I assume. — ackh, Apr 20 '22 at 07:37
@Sedenion Thinking about this, it might be the case that `_criticalSection` gets destructed before the lock is freed in the rendering loop. — ackh, Apr 20 '22 at 08:06
I now moved `concurrency::critical_section _criticalSection;` from the `BaseView` to `MyCustomView` but I'm still getting the same issue. — ackh, Apr 20 '22 at 09:06
The compiler generates the constructor and destructor automatically in your base class. So you have two distinct critical sections or what? And what do you mean with "I now moved ... to `MyCustomView`": there was a critical section with the same name already... Show us some consistent and complete code — Sedenion, Apr 20 '22 at 15:53

score 0 · Answer 1 · edited Apr 20 '22 at 04:02

0

To me it looks like the problem is that your mainthread is destructing your objects, which contains the other thread.

You are trying to solve this by setting _isRenderLoopRunning to false, but other tread will will not be synchronized with that new value. So the other thread continues running, while you destruct the object with the main thread, which gives you this error.

You could make some methods that locks the access to this variable, so it has a lock when it is read, and also for when it is changed.

edited Apr 20 '22 at 04:02

Cristik

30,989
25
91
127

answered Apr 19 '22 at 21:54

the1bird

101
1

Have you read the comments on the original post? Of course the problem is that the object is destroyed while the thread is still running. That is what the error says. But the problem appears to originate from code that ist not shown by the OP. – Sedenion Apr 20 '22 at 05:57
The problem is not that some other object is calling the destructor of MyCustomView... The intention is to destruct the object, without errors. To do that, it can be solved as stated above. – the1bird Apr 20 '22 at 20:40

How to address the "Lock was destructed while held" Visual C++ runtime assertion

1 Answers1