Garbage collected delegate in C++/CLI

Question

I have an observer pattern in C++ which I'm allowing access to from C# via a C++/CLI wrapper. I have discovered that it is not working as expected with garbage collection. I am getting Call has been made on garbage collected delegate errors, but as far as I can tell, I AM holding on to a managed reference to the delegate (via the listeners_ dictionary), so I don't understand why it's being GC'd.

Here I'm just showing the C++/CLI wrapper code, which implements the same interface as the wrapped C++ code (which for example purposes I've put in the "native" namespace).

Is there something wrong with the way I forward unmanaged updates to the managed delegate, how I hold on to managed delegate, or how I've implemented the addListener/removeListener functions?

using namespace System::Runtime::InteropServices;
using namespace System::Collections::Generic;
typedef boost::shared_ptr<native::IterationListener> IterationListenerPtr;

public ref struct IterationListener
{
    enum class Status {Ok, Cancel};

    ref struct UpdateMessage
    {
        UpdateMessage(int iterationIndex, int iterationCount, System::String^ message);

        property System::String^ message;
        property int iterationIndex;
        property int iterationCount;
    };

    IterationListener();

    virtual Status update(UpdateMessage^ updateMessage) {return Status::Ok;}
};

public delegate IterationListener::Status IterationListenerUpdate(IterationListener::UpdateMessage^ updateMessage);


#define DEFINE_INTERNAL_BASE_CODE(CLIType, NativeType) \
public:   System::IntPtr void_base() {return (System::IntPtr) base_;} \
internal: CLIType(NativeType* base, System::Object^ owner) : base_(base), owner_(owner) {} \
          CLIType(NativeType* base) : base_(base), owner_(nullptr) {} \
          virtual ~CLIType() {if (owner_ == nullptr) {SAFEDELETE(base_);}} \
          !CLIType() {delete this;} \
          NativeType* base_; \
          System::Object^ owner_; \
          NativeType& base() {return *base_;}


public ref class IterationListenerRegistry
{
    DEFINE_INTERNAL_BASE_CODE(IterationListenerRegistry, native::IterationListenerRegistry);
    System::Collections::Generic::Dictionary<IterationListener^,
                                             KeyValuePair<IterationListenerUpdate^, System::IntPtr> >^ _listeners;

    public:

    IterationListenerRegistry();

    void addListener(IterationListener^ listener, System::UInt32 iterationPeriod);
    void addListenerWithTimer(IterationListener^ listener, double timePeriod); // seconds
    void removeListener(IterationListener^ listener);

    IterationListener::Status broadcastUpdateMessage(IterationListener::UpdateMessage^ updateMessage);
};






IterationListener::IterationListener()
{
}


IterationListener::UpdateMessage::UpdateMessage(int iterationIndex,
                                                int iterationCount,
                                                System::String^ message)
{
    this->iterationIndex = iterationIndex;
    this->iterationCount = iterationCount;
    this->message = message;
}


struct IterationListenerForwarder : public native::IterationListener
{
    typedef IterationListener::Status (__stdcall *IterationListenerCallback)(IterationListener::UpdateMessage^);
    IterationListenerCallback managedFunctionPtr;

    IterationListenerForwarder(void* managedFunctionPtr)
        : managedFunctionPtr(static_cast<IterationListenerCallback>(managedFunctionPtr))
    {}

    virtual Status update(const UpdateMessage& updateMessage)
    {
        if (managedFunctionPtr != NULL)
        {
            IterationListener::UpdateMessage^ managedUpdateMessage =
                gcnew IterationListener::UpdateMessage(updateMessage.iterationIndex,
                                                       updateMessage.iterationCount,
                                                       ToSystemString(updateMessage.message));
            return (Status) managedFunctionPtr(managedUpdateMessage);
        }

        return Status_Ok;
    }
};


IterationListenerRegistry::IterationListenerRegistry()
{
    base_ = new native::IterationListenerRegistry();
    _listeners = gcnew Dictionary<IterationListener^, KeyValuePair<IterationListenerUpdate^, System::IntPtr> >();
}


void IterationListenerRegistry::addListener(IterationListener^ listener, System::UInt32 iterationPeriod)
{
    IterationListenerUpdate^ handler = gcnew IterationListenerUpdate(listener, &IterationListener::update);
    IterationListenerPtr forwarder(new IterationListenerForwarder(Marshal::GetFunctionPointerForDelegate(handler).ToPointer()));
    _listeners->Add(listener, KeyValuePair<IterationListenerUpdate^, System::IntPtr>(handler, System::IntPtr(&forwarder)));
    base().addListener(forwarder, (size_t) iterationPeriod);
}


void IterationListenerRegistry::addListenerWithTimer(IterationListener^ listener, double timePeriod)
{
    IterationListenerUpdate^ handler = gcnew IterationListenerUpdate(listener, &IterationListener::update);
    IterationListenerPtr forwarder(new IterationListenerForwarder(Marshal::GetFunctionPointerForDelegate(handler).ToPointer()));
    _listeners->Add(listener, KeyValuePair<IterationListenerUpdate^, System::IntPtr>(handler, System::IntPtr(&forwarder)));
    base().addListenerWithTimer(forwarder, timePeriod);
}


void IterationListenerRegistry::removeListener(IterationListener^ listener)
{
    base().removeListener(*static_cast<native::IterationListenerPtr*>(_listeners[listener].Value.ToPointer()));
    _listeners->Remove(listener);
}


IterationListener::Status IterationListenerRegistry::broadcastUpdateMessage(IterationListener::UpdateMessage^ updateMessage)
{
    std::string message = ToStdString(updateMessage->message);
    native::IterationListener::UpdateMessage nativeUpdateMessage(updateMessage->iterationIndex,
                                                            updateMessage->iterationCount,
                                                            message);
    return (IterationListener::Status) base().broadcastUpdateMessage(nativeUpdateMessage);
}

Answer 1

The problem was indeed in:

IterationListenerUpdate^ handler = gcnew IterationListenerUpdate(listener, &IterationListener::update);
IterationListenerPtr forwarder(new IterationListenerForwarder(Marshal::GetFunctionPointerForDelegate(handler).ToPointer()));
_listeners->Add(listener, KeyValuePair<IterationListenerUpdate^, System::IntPtr>(handler, System::IntPtr(&forwarder)));
base().addListener(forwarder, (size_t) iterationPeriod);

I was creating a shared_ptr (IterationListenerPtr) on the stack and keeping the address of it in the _listeners dictionary. As soon as the shared_ptr goes out of scope, that address is no longer valid. What probably prevented all hell from breaking loose is the fact that the shared_ptr's contents were being kept alive by passing it to the native code (which holds on to a copy of the shared_ptr, not the address of it).

However, I'm still not sure why this didn't trigger some kind of alert when I was running in full debug mode. I'm still not sure why it triggered the delegate-has-been-garbage-collected MDA when I'm pretty sure that wasn't the issue. It seems to me it's exactly the kind of error that using a debug allocator and native run-time checks is supposed to find. :(

The fix was easy:

IterationListenerUpdate^ handler = gcnew IterationListenerUpdate(listener, &IterationListener::update);
IterationListenerPtr* forwarder = new IterationListenerPtr(new IterationListenerForwarder(Marshal::GetFunctionPointerForDelegate(handler).ToPointer()));
_listeners->Add(listener, KeyValuePair<IterationListenerUpdate^, System::IntPtr>(handler, System::IntPtr(forwarder)));
base().addListener(*forwarder, (size_t) iterationPeriod);

I just have to make sure to delete it in the destructor or in removeListener().

Answer 2

IterationListenerUpdate^ handler = gcnew IterationListenerUpdate(listener, &IterationListener::update);
IterationListenerPtr forwarder(new IterationListenerForwarder(Marshal::GetFunctionPointerForDelegate(handler).ToPointer()));

Bit much to plow through but I think it's this code. Your handler variable is a local variable of the method and stores the delegate object. You then pass a thunk initialized from it to native code. Right after this, there's no live reference left to that delegate object. So the next GC is going to collect it. And that breaks the thunk when it tries to make the callback.

You'll need to store handler in a place where the GC can see it. Like a field of your class (assuming the class object lives long enough) or a static variable. And you must ensure it stays visible as long as the native code can make callbacks.