The following forum(s) are migrating to a new home on Microsoft Q&A (Preview): Developing Universal Windows apps!

Ask new questions on Microsoft Q&A (Preview).
Interact with existing posts until December 13, 2019, after which content will be closed to all new and existing posts.

Learn More

 none
[UWP][CX]C++/CX Destructor not called RRS feed

  • Question

  • I created a ref class Dispatcher for our WinRT application that uses a thread from the Windows::System::Threading::ThreadPool to create sort of a message pump infrastructure. The Dispatcher must be inherited in order for the derived class have this mechanism.

    The problem is that every class that derives from this base Dispatcher is not destructed (D'tor not called).

    I isolated the problem and I think I have an understanding of what causes this issue, but I'm not sure how to solve this.

    Here's some of the code that relates to the issue:

    public delegate void FunctionDelegate();
    ref class Dispatcher
    {
    protected private:
        Dispatcher()
        {   
            m_invocationHandle = CreateEvent(nullptr, FALSE, FALSE, nullptr);
            m_disposed = false;
    
            m_asyncThread = Windows::System::Threading::ThreadPool::RunAsync(
                ref new Windows::System::Threading::WorkItemHandler(
                    [this](Windows::Foundation::IAsyncAction^ operation)
            {
                while (m_disposed == false)
                {
                    WaitForSingleObject(m_invocationHandle, INFINITE);
                    //copy Pending Queue to Executing Queue
                    //Run all handlers in Executing Queue and clear it
                }
            }));
        }
    
    public:
        virtual ~Dispatcher()
        {
            m_disposed = true;
            SetEvent(m_invocationHandle);
            JoinInvocationThread();
            CleanUp(); //close handles etc...
        }
    
        void BeginInvoke(FunctionDelegate^ function)
        {
            PendingQueue->Append(function);
            SetEvent(m_invocationHandle);
        }
    };

    So, since this is a ref class its d'tor should be invoked when the ref count reaches 0, but since I pass this to the WorkItemHandler delegate, the thread holds a reference to the Dispatcher class, which causes a circular reference. Thus since the thread is infinitely waiting for a the m_invocationHandleevent to be set, there is always a reference to this class which will never call its destructor (which should set the m_invocationHandle event and wait for the thread to complete).

    I thought about using Platform::WeakReference but I will have to Resolve it to a Dispatcher^before callsing WaitForSingleObject(...) in order to get the m_invocationHandle which doesn't help since this will raise the ref count as well.

    Any ideas?



    • Edited by ShaZiv Tuesday, July 26, 2016 6:42 AM
    • Edited by Barry Wang Wednesday, July 27, 2016 8:08 AM title tag
    Tuesday, July 26, 2016 6:41 AM

Answers

  • 1) This is C++/CX not standard C++.

    https://msdn.microsoft.com/en-us/library/hh699870.aspx

    2/3) "using it for each class' purpose" is equivalent to specializing it.You can obtain the same functionality by redirecting the call to "begin invoke" through the dispatcher member of a class.

    Either way, you still can't fix the problem "automatically" as long as you pass "this" around.


    • Edited by mcosmin Tuesday, July 26, 2016 7:45 AM
    • Proposed as answer by Barry Wang Friday, August 5, 2016 8:11 AM
    • Marked as answer by Barry Wang Monday, August 8, 2016 2:16 AM
    Tuesday, July 26, 2016 7:41 AM
  • 1) I stand corrected. Missed that one.

    I can workaround this issue by passing a reference to the required members instead of passing "this". What do you think of this solution?

    • Proposed as answer by Barry Wang Friday, August 5, 2016 8:11 AM
    • Marked as answer by Barry Wang Monday, August 8, 2016 2:16 AM
    Tuesday, July 26, 2016 7:48 AM
  • Should work if you clear the references properly when the thread is done. This will be somewhat tricky.
    • Proposed as answer by Barry Wang Friday, August 5, 2016 8:11 AM
    • Marked as answer by Barry Wang Monday, August 8, 2016 2:16 AM
    Tuesday, July 26, 2016 8:00 AM

All replies

  • You can implement the IClosable pattern and provide with a "close" method which does clean up. As long as you pass "this" around, automated GC won't work, so you have to use manual memory management.

    Having a message pomp in a class hierarchy is a stinky design. The dispatcher should be a property of a class, not a base class.


    • Edited by mcosmin Tuesday, July 26, 2016 6:53 AM
    Tuesday, July 26, 2016 6:47 AM
  • Why would you say that it is a stinky design? 
    Tuesday, July 26, 2016 6:51 AM
  • 1) It prevents you from inheriting other classes.

    2) It gives a class functionality that should not be its own.

    3) The dispatcher is a general class, thus inheriting it to make it "specialized" does not make sense, and even if you wanted to do so, it is safer to use templates/generics instead, for type safety.

    Tuesday, July 26, 2016 6:56 AM
  • 1) It prevents you from inheriting other classes.

    2) It gives a class functionality that should not be its own.

    3) The dispatcher is a general class, thus inheriting it to make it "specialized" does not make sense, and even if you wanted to do so, it is safer to use templates/generics instead, for type safety.

    1) How? C++ supports multiple inheritance

    2-3) The purpose of this class is to allow any class the functionality of a "BegingInvoke" we can argue whether or not it should be public or protected but if a class chooses to inherit from the Dispatcher, it means this class requires this functionality. It is a utility, therefore indeed it is a general class. The purpose of inheritance here is not to extend the dispatcher's functionality or "specialize" it, but to use it for each class' purpose. 

    Tuesday, July 26, 2016 7:15 AM
  • 1) This is C++/CX not standard C++.

    https://msdn.microsoft.com/en-us/library/hh699870.aspx

    2/3) "using it for each class' purpose" is equivalent to specializing it.You can obtain the same functionality by redirecting the call to "begin invoke" through the dispatcher member of a class.

    Either way, you still can't fix the problem "automatically" as long as you pass "this" around.


    • Edited by mcosmin Tuesday, July 26, 2016 7:45 AM
    • Proposed as answer by Barry Wang Friday, August 5, 2016 8:11 AM
    • Marked as answer by Barry Wang Monday, August 8, 2016 2:16 AM
    Tuesday, July 26, 2016 7:41 AM
  • 1) I stand corrected. Missed that one.

    I can workaround this issue by passing a reference to the required members instead of passing "this". What do you think of this solution?

    • Proposed as answer by Barry Wang Friday, August 5, 2016 8:11 AM
    • Marked as answer by Barry Wang Monday, August 8, 2016 2:16 AM
    Tuesday, July 26, 2016 7:48 AM
  • Should work if you clear the references properly when the thread is done. This will be somewhat tricky.
    • Proposed as answer by Barry Wang Friday, August 5, 2016 8:11 AM
    • Marked as answer by Barry Wang Monday, August 8, 2016 2:16 AM
    Tuesday, July 26, 2016 8:00 AM