September, 2008

Larry Osterman's WebLog

Confessions of an Old Fogey
  • Larry Osterman's WebLog

    I'm So Excited!

    • 34 Comments

    image

    'nuf said.

     

     

    I know it's bragging, but I'm sorry - I really couldn't resist.  It's been a very long time.

  • Larry Osterman's WebLog

    What’s wrong with this code, Part 23..

    • 26 Comments

    I recently tracked down a bug that was causing problems in my code.  Once I figured out the bug, I realized it made a good “what’s wrong with this code”…

    #include "stdafx.h"
    #include <mmdeviceapi.h>
    
    
    IMMDeviceEnumerator *GetDeviceEnumerator()
    {
        CComPtr<IMMDeviceEnumerator> deviceEnumerator;
    
        HRESULT hr = deviceEnumerator.CoCreateInstance(__uuidof(MMDeviceEnumerator));
        if (FAILED(hr))
        {
            return NULL;
        }
        return deviceEnumerator.Detach();
    
    }
    int _tmain(int argc, _TCHAR* argv[])
    {
        CoInitialize(NULL);
        {
            CComPtr<IMMDeviceEnumerator> deviceEnumerator(GetDeviceEnumerator());
            CComPtr<IMMDeviceCollection> deviceCollection;
    
            if (deviceEnumerator != NULL)
            {
                HRESULT hr = deviceEnumerator->EnumAudioEndpoints(eAll, DEVICE_STATE_ACTIVE, &deviceCollection);
                if (SUCCEEDED(hr))
                {
                    UINT deviceCount;
                    hr = deviceCollection->GetCount(&deviceCount);
                    if (SUCCEEDED(hr))
                    {
                        printf("There are %d audio endpoints on the machine\n", deviceCount);
                    }
                }
            }
        }
        CoUninitialize();
        return 0;
    }

    Simple code, right?  But there’s a nasty bug hidden in there, and it was NOT obvious to me what the bug was.

    As always, kudos to the person who gets the bug first.  And of course mea culpa's for bugs I accidentally included.

  • Larry Osterman's WebLog

    What’s wrong with this code, Part 23 – The Answers

    • 21 Comments

    My last post was all about a problem with what appeared to be some really simple ATL code.

    It turns out that the problem was easier than I had expected.  James Skimming came up with the answer on the second comment.  The problem here was that the following code (snipped a bit)

    IMMDeviceEnumerator *GetDeviceEnumerator()
    {
        CComPtr<IMMDeviceEnumerator> deviceEnumerator;
    
        HRESULT hr = deviceEnumerator.CoCreateInstance(__uuidof(MMDeviceEnumerator));
        if (FAILED(hr))
        {
            return NULL;
        }
        return deviceEnumerator.Detach();
    
    }
    int _tmain(int argc, _TCHAR* argv[])
    {
        CoInitialize(NULL);
        {
            CComPtr<IMMDeviceEnumerator> deviceEnumerator(GetDeviceEnumerator());
            CComPtr<IMMDeviceCollection> deviceCollection;
    
            if (deviceEnumerator != NULL)
            {
                HRESULT hr = deviceEnumerator->EnumAudioEndpoints(eAll, DEVICE_STATE_ACTIVE, &deviceCollection);
                if (SUCCEEDED(hr))
                {

    That’s because the CComPtr<T> constructor takes a reference to the input T* object.  As a result, the code leaks a reference to the MMDeviceEnumerator object.  The correct fix for the problem generated a fair amount of discussion in the comments and on email internally.

    The root cause of the problem  is that the code in question mixes raw interface pointers and smart pointers.  In this case there’s an impedance mismatch between the GetDeviceEnumerator (which returns a raw pointer) and it’s caller (which is expecting a smart pointer).

    My preferred solution to the problem is:

    CComPtr<IMMDeviceEnumerator> GetDeviceEnumerator()
    {
        CComPtr<IMMDeviceEnumerator> deviceEnumerator;
    
        HRESULT hr = deviceEnumerator.CoCreateInstance(__uuidof(MMDeviceEnumerator));
        if (FAILED(hr))
        {
            return NULL;
        }
        return deviceEnumerator;
    
    }

    The reason I prefer this solution is that it consistently uses the smart pointer class.  There are two downsides to it.  The first is that it loses the result of the call to CoCreateInstance.  The other downside is that it constructs a temporary object on the stack and has two additional calls to AddRef()/Release().

     

    There are other possible solutions.  The second possible solution changes the GetDeviceEnumerator function:

    HRESULT GetDeviceEnumerator(IMMDeviceEnumerator *Enumerator)
    {
        CComPtr<IMMDeviceEnumerator> deviceEnumerator;
    
        if (Enumerator == NULL)
        {
            return E_POINTER;
        }
        HRESULT hr = deviceEnumerator.CoCreateInstance(__uuidof(MMDeviceEnumerator));
        if (FAILED(hr))
        {
            *Enumerator = NULL;
            return hr;
        }
        *Enumerator = deviceEnumerator.Detach();
    
        return S_OK;
    }

    With this change, the caller looks like:

        {
            CComPtr<IMMDeviceEnumerator> deviceEnumerator;
            if (SUCCEEDED(GetDeviceEnumerator(&deviceEnumerator))
            {
                CComPtr<IMMDeviceCollection> deviceCollection;
                HRESULT hr = deviceEnumerator->EnumAudioEndpoints(eAll, DEVICE_STATE_ACTIVE, &deviceCollection);
                if (SUCCEEDED(hr))

    This solution fits closely with the COM usage pattern so it is quite attractive.  You also don’t lose the result of the CoCreateInstance API call, which can be extremely useful for diagnostics.  The major negative with this is that it depends on the fact that the CComPtr<T> operator& returns a raw pointer to the underlying object.

     

    The third possible solution keeps the “return a value” idea of the original code but works around the additional reference applied in the constructor.  Keep the original implementation of GetDeviceEnumerator and change the tmain function as follows:

    int _tmain(int argc, _TCHAR* argv[])
    {
        CoInitialize(NULL);
        {
            CComPtr<IMMDeviceEnumerator> deviceEnumerator;
            deviceEnumerator.Attach(GetDeviceEnumerator());
            CComPtr<IMMDeviceCollection> deviceCollection;

    The big problem with this solution is that to me it’s “unnatural” – the whole point of using smart pointers is that their use is supposed to be intuitive, however in this case there’s nothing intuitive about the use – it certainly fixes the problem but it relies on internal behaviors of the smart pointer class.  To me this is the least attractive solution to the problem.

     

    As I mentioned, Kudos to James Skimming for figuring the problem out quickly.

  • Larry Osterman's WebLog

    Anatomy of a Heisenbug

    • 15 Comments

    I just spent a half an hour debugging a heisenbug and thought I’d pass on what was happening.

     

    I was running my unit tests for one of my features and they were reliably failing.  Unfortunately the instant I ran the test case under the debugger, the problem went away.  Problems that disappear under the debugger are a classic symptom of a heisenbug, this was no exception.

    If I attached the debugger AFTER the test started but before the failure hit, I was able to see the failure occur.  The problem only occurred when launching the app under the debugger.  At that point I realized what was happening.

    As MSDN says

    Processes that the debugger creates (also known as spawned processes) behave slightly differently than processes that the debugger does not create.

    Instead of using the standard heap API, processes that the debugger creates use a special debug heap. On Microsoft Windows XP and later versions of Windows, you can force a spawned process to use the standard heap instead of the debug heap by using the _NO_DEBUG_HEAP environment variable or the -hd command-line option.

    It turns out that I had added a member variable to a class and failed to initialize it in the constructor of the class.  When launched under the debugger, the debug heap initializes all allocated memory to a known value.  That means that when launched under the debugger the member variable had that known value, when launched without the debugger it was uninitialized and happened to have the value of 0.  For a number of reasons, this value caused the test to fail. 

     

    I hate heisenbugs, but I like it when they’re really easy to find like this one.

  • Larry Osterman's WebLog

    Why call PlaySound(NULL, NULL, SND_NODEFAULT)?

    • 5 Comments

    Someone just wandered over to my office and he had noticed the following pattern in his code:

    PlaySound(NULL, NULL, SND_NODEFAULT);
    PlaySound(".Default", NULL, SND_SYSTEM | SND_ASYNC | SND_NODEFAULT);

    He was wondering why on earth the code would do that call to PlaySound(NULL). 

    As I explained it to him, the reason is because you almost always want to cancel the current sound playing before you queue up the next sound.

    The problem is that the current implementation[1] of PlaySound(…, SND_ASYNC) simply queues the request to a worker thread which blocks waiting on the currently playing sound to complete.  So if you have a situation where you call PlaySound(…, SND_ASYNC) many times in succession, you’ll find that all the calls to PlaySound pile up behind each other, which means that you might end up playing sounds long after the action associated with the sound has completed.

    Of course you might want this behavior – it’s certainly possible to string lots of system sounds together to produce any number of interesting effects.

    But most of the time you just want to stop the current sound before you start playing the next.

     

     

    [1] There are obviously no guarantees that the implementation of PlaySound won’t change – I’m just describing what the current code does.  Even if the implementation is changed, it won’t change the underlying behavior.

  • Larry Osterman's WebLog

    They just announced my PDC talk, so I guess I can mention at least the title of the talk.

    • 5 Comments

    The PDC folks just announced a host of Windows 7 related PDC talks, one of which is mine.

    The title of the talk is “Windows 7: Building Great Communications Applications”.  You can find it under the Windows 7 track on the Microsoft PDC site.

     

    The primary target for my talk is developers who are building an application that in any way communicates between users (voice mail, instant messaging, voice over ip, etc).  In addition, if you’re a games developer or a media player developer, you should also attend, there’s stuff in the talk for you too.

     

    There are also some other cool talks included in the list that I’m absolutely planning on attending.

     

     

    See you in LA!

  • Larry Osterman's WebLog

    Why specify SND_NODEFAULT when calling PlaySound?

    • 5 Comments

    Because the alternative is often much worse.

     

    Several months ago, I got a bug report that if you launched mmsys.cpl then set the “Select” sound to a value, then cleared the sound, the reporters application would ding whenever you moved around their tree control.

    I dug into the problem a bit and discovered that the problem was actually in the Windows common controls.  Under some circumstances the common controls would call PlaySound specifying a sound alias and the SND_ASYNC and SND_ALIAS flags only. 

    The problem with this is that if you specify SND_ALIAS without also specifying SND_NODEFAULT, the PlaySound API decides that you really want the sound to be played and it will play the default “ding” sound instead. 

    From the PlaySound API’s point of view this makes sense.  After all, you asked the API to play a sound, it doesn’t know that you meant “only play this sound when the sound file represented by the alias exists”. 

     

    In fact, that’s the entire reason for the SND_NODEFAULT flag – it lets the PlaySound API that you only want to play a sound when the sound has been defined.

  • Larry Osterman's WebLog

    Ok, I’m strange…

    • 5 Comments

    I was looking at the front page of MSNBC and ran into this picture:

    Image: Traders at stock exchange in Sao Paulo, Brazil

    (image copyright Sebastiao Moreira / EPA). 

     

    My first thought was “Why do these people all have hotdogs with yellow mustard pressed up to their face?”

     

    I did say that I was being weird.

  • Larry Osterman's WebLog

    See you in LA in October!

    • 3 Comments

    I just got word that the talk we’d proposed for the PDC in October was approved, so I’m going to La-La-Land next month for a couple of days.

     

    The contents of my talk have not yet been disclosed, so I can’t talk about what I’m talking about at the PDC :(. 

     

    Disclaimer: Life sometimes happens.  Right now I’m scheduled to talk but who knows whats gonna happen between now and then?  But I AM excited.  This will be my second PDC – the first one was WAY back in 1992 (I need to make sure that I remember to bring my speaker badge from that (yes, I still have it)).

  • Larry Osterman's WebLog

    DrawThemeText doesn’t really support the DT_CALCRECT format value

    • 1 Comments

    I just noticed this while working on a bug fix.

    We have some code that attempts to determine the size of a piece of text by calling DrawThemeText with the DT_CALCRECT format value.

    According to this page (as of 9/30/2008) the DT_CALCRECT option determines the width and height of the display rectangle.

    Unfortunately the DT_CALCRECT option doesn’t actually return the width and height of the display rectangle.  Instead it doesn’t modify the input LPRECT field.  I asked the developer and he indicated that the reason was that the pRect parameter to DrawThemeText is actually defined as a LPCRECT and thus the implicit contract for the API is that it doesn’t change the contents of the pRect field.

     

    There are two ways of working around this problem.  The first (and recommended) way is to call GetThemeTextExtent to retrieve the dimensions of the rectangle.  The second (which works on Vista and later operating systems) is to call DrawThemeTextEx passing in the DTT_CALCRECT flag – in that case the DrawThemeTextEx API will return the size of the text in the pRect parameter.

     

    I’ve sent emails to the SDK folks so it’s entirely likely that the page will be updated in the future but until it is, hopefully this will help someone using the search engines to find what’s going on.

Page 1 of 1 (10 items)