What's the purpose of Lock()/Unlock() calls when firing COM events?

Question

A typical piece of code firing events in an ATL COM server goes like this (copied from this question and trimmed a bit):

HRESULT Fire_MessageTrigger()
{
    HRESULT hr = S_OK;
    T * pThis = static_cast<T *>(this);
    int count = m_vec.GetSize();

    for (int i = 0; i < count; i++)
    {
        pThis->Lock(); // I'm asking about this...
        CComPtr<IUnknown> punkConnection = m_vec.GetAt(i);
        pThis->Unlock(); // and this

        IDispatch* pConnection = static_cast<IDispatch *>(punkConnection.p);
        if (pConnection == 0)
            continue;

        DISPPARAMS params = { NULL, NULL, 0, 0 };
        hr = pConnection->Invoke(2, IID_NULL, LOCALE_USER_DEFAULT,
            DISPATCH_METHOD, &params, 0, NULL, NULL);
    }
    return hr;
}

What's the purpose of the Lock() and Unlock() calls?

Answer 1

It is thread safety measure around m_vec member variable (list of sinks).

You can have the following being executed on a concurrent thread:

template <class T, const IID* piid, class CDV>
STDMETHODIMP IConnectionPointImpl<T, piid, CDV>::Advise(
    _Inout_ IUnknown* pUnkSink,
    _Out_ DWORD* pdwCookie)
{
// ...
        pT->Lock();
        *pdwCookie = m_vec.Add(p); // <<--- Modifying m_vec here
        hRes = (*pdwCookie != NULL) ? S_OK : CONNECT_E_ADVISELIMIT;
        pT->Unlock();

Your next question would be why they are not locking a few lines above around accessing m_vec.GetSize();.

This is the way they did it: they are OK to miss an individual event delivery in case of concurrent vector access, as long as it does not create memory corruption or otherwise undefined behavior. After all, the subscribing call could happen a moment later and miss this event anyways. The same applies to the case of unsubscription from events (Unadvise).

The priority here is to lock as rare as possible, and then unlock as soon as possible. Note that sink call takes place while unlocked, and sink list can take a modification while we're iterating through the list of sinks making calls on them.

UPD. Visual Studio 2008 always generates Lock/Unlock pair. So do newer versions. From "\VC\VCWizards\CodeWiz\ATL\ImplementInterface\HTML\1033\default.htm" file:

strProxyMethod +=
    "\t\tfor (int iConnection = 0; iConnection < cConnections; iConnection++)\r\n"+
    "\t\t{\r\n"+
    "\t\t\tpThis->Lock();\r\n"+
    "\t\t\tCComPtr<IUnknown> punkConnection = m_vec.GetAt(iConnection);\r\n"+
    "\t\t\tpThis->Unlock();\r\n\r\n";

Earlier VS versions could generate lock-free code and the code is only updated when you re-generate proxy class manually from VS IDE.