Getting a strong reference from the this pointer too late

Raymond Chen

It is a standard pattern for functions that are coroutines to promote the this pointer to a strong reference (either a COM strong reference or a shared_ptr), so that the object won’t be destructed while the coroutine is suspended. But it might be too late.

Consider the following example:

struct MyObject : winrt::implements<MyObject, winrt::IInspectable>
{
  MyObject() = default;
  ~MyObject() = default;

  winrt::Widget::Closed_revoker m_revoker;

  void RegisterForWidgetEvents(Widget const& widget)
  {
      m_revoker = widget.Closed(winrt::auto_revoke,
        { this, &MyObject::OnWidgetClosed });
  }

  winrt::fire_and_forget OnWidgetClosed(Widget const& sender, winrt::IInspectable const&)
  {
    auto lifetime = get_strong();

    co_await DoStuffAsync();
    co_await DoMoreStuffAsync();
  }
};

The idea here is that we register for the Widget’s Closed event with a raw pointer. When the event is raised, the handle immediately promotes the raw pointer to a strong reference, so that the MyObject does not destruct during the two asynchronous calls that follow.

But there’s still a race condition:

Thread 1 Thread 2
  Widget closes
Last reference released  
  Widget::OnWidgetClosed begins
Destruction begins  
m_revoker unregisters handler  
  get_strong()

If the last reference is released before the Widget::On­Widget­Closed method reaches the get_strong(), then the get_strong() method runs against an object that has already started destructing. It will nevertheless produce a strong reference and increment the reference count, but that reference count does not have the power of time travel. The destructor is already running; you incremented the reference count too late. The result is a mysterious crash.

A similar problem exists with std::shared_ptr:

struct MyObject : std::enable_shared_from_this<MyObject>
{
  MyObject() = default;
  ~MyObject() = default;

  winrt::Widget::Closed_revoker m_revoker;

  void RegisterForWidgetEvents(Widget const& widget)
  {
      m_revoker = widget.Closed(winrt::auto_revoke,
        { this, &MyObject::OnWidgetClosed });
  }

  winrt::fire_and_forget OnWidgetClosed(Widget const& sender, winrt::IInspectable const&)
  {
    auto lifetime = shared_from_this();

    co_await DoStuffAsync();
    co_await DoMoreStuffAsync();
  }
};
Thread 1 Thread 2
  Widget closes
Last reference released  
  Widget::OnWidgetClosed begins
Destruction begins  
m_revoker unregisters handler  
  shared_from_this()

The call to shared_from_this() throws std::bad_weak_ptr because the weak pointer cannot be converted to a shared_ptr.

In both cases, the problem is that the On­Widget­Closed callback is registered with a raw pointer. Instead, use a weak pointer and try to promote it to a strong pointer in the callback.

  // C++/WinRT
  void RegisterForWidgetEvents(Widget const& widget)
  {
    m_revoker = widget.Closed(winrt::auto_revoke,
      [weak = get_weak()](auto&& sender, auto&& args)
      {
        if (auto strong = weak.get()) {
          strong->OnWidgetClosed(sender, args);
        }
      });
  }

  // C++/WinRT alternate version
  void RegisterForWidgetEvents(Widget const& widget)
  {
    m_revoker = widget.Closed(winrt::auto_revoke,
      { get_weak(), &MyObject::OnWidgetClosed });
  }

  // C++ standard library
  void RegisterForWidgetEvents(Widget const& widget)
  {
    m_revoker = widget.Closed(winrt::auto_revoke,
      [weak = weak_from_this()](auto&& sender, auto&& args)
      {
        if (auto strong = weak.lock()) {
          strong->OnWidgetClosed(sender, args);
        }
      });
  }

C++/WinRT provides a helper constructor that does the auto strong = weak.get() thing automatically.

Since weak pointers will not promote to strong/shared pointers once the last strong/shared reference is destructed, you don’t have the race condition where the callback tries to do something with an object that has begun destructing.

Thread 1 Thread 2
  Widget closes
Last reference released
(weak pointers are now expired)
 
  Widget::OnWidgetClosed begins
Destruction begins  
m_revoker unregisters handler  
  weak.get() fails

3 comments

Discussion is closed. Login to edit/delete existing comments.

  • 紅樓鍮 0

    I think it should be clarified that the “Last reference released” under “Thread 1” refers to the last reference to MyObject being released, which by nature is unrelated to the Widget being closed. This could have been less confusing if the Widget did some other thing like Triggered or Ticked; the current wording of the problem makes it sound like the closing of the Widget somehow causes of the ending of the lifetime of MyObject, which, even if it’s true, is not central to the problem.

  • Neil Rashbrook 0

    I thought (possibly erroneously) that it was a requirement of COM that the caller was required to have a strong reference to the object. How is it possible that you can call Widget::OnWidgetClosed on a widget that has already had its last reference released?

    • Raymond ChenMicrosoft employee 0

      COM totally had a strong reference to the delegate. But the delegate has a raw pointer to the MyObject, via { this, &MyObject::OnWidgetClosed }. It is the delegate that caused the problem, not COM.

Feedback usabilla icon