RTM Credential Providers – Issue with AddRef().


If you are using the RTM Credential Providers Sample for Windows Vista you might see that the destructors are never called when you do a logoff/ logon or a lock/ unlock or restart of the workstation.

I was using the SDK “Sample Credential Provider” code (from “\Microsoft SDKs\Windows\v7.0\Samples\security\credentialproviders”), did an extensive logging to make sure how the various functions are called when we do a logoff/ logon, lock/ unlock, and restart of the Win7 and Vista OS after the sample credential provider is registered. I do see the call sequence below.

1  : CSample_CreateInstance()

2  : CSampleProvider::CSampleProvider()

3  : CSampleProvider::SetUsageScenario()

4  : CSampleProvider::_EnumerateCredentials()

5  : CSampleProvider::_EnumerateOneCredential()

6  : CSampleCredential::CSampleCredential()

7  : CSampleCredential::Initialize()

8  : CSampleProvider::_EnumerateOneCredential()

9  : CSampleCredential::CSampleCredential()

10  : CSampleCredential::Initialize()

11  : CSampleProvider::Advise()

12  : CSampleProvider::GetCredentialCount()

13  : CSampleProvider::GetCredentialAt()

14  : CSampleProvider::GetCredentialAt()

15  : CSampleProvider::GetFieldDescriptorCount()

16  : CSampleProvider::GetFieldDescriptorAt()

17  : CSampleProvider::GetFieldDescriptorAt()

18  : CSampleProvider::GetFieldDescriptorAt()

19  : CSampleProvider::GetFieldDescriptorAt()

20  : CSampleCredential::GetBitmapValue()

21  : CSampleCredential::GetFieldState()

22  : CSampleCredential::GetStringValue()

23  : CSampleCredential::GetFieldState()

24  : CSampleCredential::GetStringValue()

25  : CSampleCredential::GetFieldState()

26  : CSampleCredential::GetSubmitButtonValue()

27  : CSampleCredential::GetFieldState()

28  : CSampleCredential::GetBitmapValue()

29  : CSampleCredential::GetFieldState()

30  : CSampleCredential::GetStringValue()

31  : CSampleCredential::GetFieldState()

32  : CSampleCredential::GetStringValue()

33  : CSampleCredential::GetFieldState()

34  : CSampleCredential::GetSubmitButtonValue()

35  : CSampleCredential::GetFieldState()

36  : CSampleCredential::Advise()

37  : CSampleCredential::SetSelected()

38  : CSampleCredential::UnAdvise()

39  : CSampleCredential::Advise()

40  : CSampleCredential::SetStringValue()

41  : CSampleCredential::UnAdvise()

42  : CSampleCredential::Advise()

43  : CSampleCredential::SetStringValue()

44  : CSampleCredential::UnAdvise()

45  : CSampleCredential::Advise()

46  : CSampleCredential::SetStringValue()

47  : CSampleCredential::UnAdvise()

48  : CSampleCredential::Advise()

49  : CSampleCredential::SetStringValue()

50  : CSampleCredential::UnAdvise()

51  : CSampleCredential::Advise()

52  : CSampleCredential::SetStringValue()

53  : CSampleCredential::UnAdvise()

54  : CSampleCredential::Advise()

55  : CSampleCredential::GetSerialization()

56  : CSampleCredential::UnAdvise()

57  : CSampleProvider::UnAdvise()

58  : CSampleCredential::Advise()

59  : CSampleCredential::ReportResult()

60  : CSampleCredential::UnAdvise()

61  : CSampleProvider::UnAdvise()

62  : CSampleProvider::~CSampleProvider()

63  : CSampleCredential::~CSampleCredential()

64  : CSampleCredential::~CSampleCredential()

 

The last 3 calls are of specific interest to me. You can see that the respective destructors are called; but when I am using the RTM Credential providers (from http://www.microsoft.com/downloads/details.aspx?FamilyID=b1b3cbd1-2d3a-4fac-982f-289f4f4b9300&displaylang=en&Hash=Su1U%2b%2fVqACdUHF46mpB7vgzLHjjrsOAAxSlyqbMb9SSgUKSVTBv6ivfRzkEIWSX6FNhAckt7q9MNCWNlJ%2f0Wsg%3d%3d), and using the same “Sample Credential Provider” code, I do see the first 61 calls but the destructors are never called. The only difference in code that I find is in the implementation of QueryInterface() in Dll.cpp.

    // From “\Microsoft SDKs\Windows\v7.0\Samples\security\credentialproviders

    IFACEMETHODIMP QueryInterface(__in REFIID riid, __deref_out void **ppv)

    {

        static const QITAB qit[] =

        {

            QITABENT(CClassFactory, IClassFactory),

            { 0 },

        };

        return QISearch(this, qit, riid, ppv);

    }

 

    IFACEMETHODIMP_(ULONG) AddRef()

    {

        return InterlockedIncrement(&_cRef);

    }

 

    IFACEMETHODIMP_(ULONG) Release()

    {

        LONG cRef = InterlockedDecrement(&_cRef);

        if (!cRef)

            delete this;

        return cRef;

    }

 

 

    // From RTM CredentialProviders

    STDMETHOD (QueryInterface)(REFIID riid, void** ppv)

    {

        HRESULT hr;

        if (ppv != NULL)

        {

            if (IID_IClassFactory == riid || IID_IUnknown == riid)

            {

                *ppv = static_cast<IUnknown*>(this);

                reinterpret_cast<IUnknown*>(*ppv)->AddRef();

                hr = S_OK;

            }

            else

            {

                *ppv = NULL;

                hr = E_NOINTERFACE;

            }

        }

        else

        {

            hr = E_INVALIDARG;

        }

        return hr;

    }

 

    STDMETHOD_(ULONG, AddRef)()

    {

        return _cRef++;

    }

   

    STDMETHOD_(ULONG, Release)()

    {

        LONG cRef = _cRef–;

        if (!cRef)

        {

            delete this;

        }

        return cRef;

    }

 

 

The problem is the following:

LONG cRef = _cRef–;

The assignment of _cRef to cRef happens before the decrement, so when _cRef goes from 1 to 0, cRef is assigned 1.

The AddRef() code has the same issue, but people rarely look at the return value of AddRef().

This can be fixed by either going back to using the Interlocked…() functions, or using “cRef = –_cRef;”.

 

Written By
Shamik Misra
Escalation Services, Microsoft Developer Support


Comments (3)

  1. Parrowdice says:

    I've just discovered this problem myself, and solved it by doing exactly the same as you. I'm not that familiar with COM, so it's good to know that it's the right thing to do. The credential provider samples really need to be improved.

  2. JFullert says:

    Using cref =- _cRef; is still incorrect. It needs to be cref = –_cRef; otherwise the value of _cRef isn’t decremented.

    1. JFullert says:

      Note the site formatting makes it look like only one minus sign in front of _cRef. It is actually two minus signs.

Skip to main content