3
\$\begingroup\$

Earlier today, I encountered an Access Violation error when opening the VBE for SolidWorks 2006, while Rubberduck VBA was initializing.

I traced the Access Violation back to an attempt to set the Picture property of a CommandBarButton, and then posted on SO

Initially, I suspected that SolidWorks might have implemented the VBE in some wacky out-of-process way, as the Picture property uses StdOle's IPictureDisp which will throw an AccessViolation on crossing a process boundary.

But upon skipping the Picture assignments, Rubberduck was able to fully load, and I discovered that SolidWorks 2006's VBE has CommandBarControl's that don't have Picture or Mask properties.

A little more digging, and it became clear that Office first introduced Picture and Mask in Office XP, but evidently SolidWorks hadn't updated the CommandBars within their VBE implementation (they possibly used an older version of the VBA SDK?).

Rubberduck is using the embedded types of the Office Interop Assembly that contains the CommandBar types, and that is from after Office XP.

So it seems that accessing the non-existent Picture and Mask properties would ordinarily cause a catchable exception, but because they involve StdOle, .NET sees a process boundary has been crossed and throws an AccessViolation.

I had to find a way of safely setting the picture, so I've elected to cast the button to a dynamic and then try reading the property. I could have just cast to dynamic and set the Picture, but it seemed to deserve its own helper function, HasPictureProperty...

    public static void SetButtonImage(CommandBarButton button, Image image, Image mask)
    {
        button.FaceId = 0;
        if (image == null || mask == null)
        {
            return;
        }

        if (!HasPictureProperty(button))
        {
            return;
        }

        try
        {
            button.Picture = AxHostConverter.ImageToPictureDisp(image);
            button.Mask = AxHostConverter.ImageToPictureDisp(mask);
        }
        catch (COMException exception)
        {
            Logger.Debug("Button image could not be set for button [" + button.Caption + "]\n" + exception);
        }
    }

    private static bool HasPictureProperty(CommandBarButton button)
    {
        try
        {
            dynamic control = button;
            object picture = control.Picture;
            return true;
        }

        catch (Microsoft.CSharp.RuntimeBinder.RuntimeBinderException exception)
        {
            Logger.Debug("Button image cannot be set for button [" + button.Caption + "], because Host VBE CommandBars are too old.\n" + exception);
        }

        return false;
    }

Am I abusing dynamic in this instance? Should I have perhaps have opted for reflection over catching what might be many RuntimeBinderException's?

BTW - The upside of this fix will mean that Rubberduck VBA should now support Office 2000 and earlier, albeit the Rubberduck toolbar buttons won't have images.

\$\endgroup\$
1
\$\begingroup\$

SetButtonImage is called for every single CommandMenuItem in Rubberduck, regardless of whether or not Picture and Mask are overridden.

Whether a CommandBarButton has a Picture property depends on the [version of the] host application, and the bool result of HasPictureProperty will be the same for every single CommandMenuItem being initialized: if a RuntimeBinderException is thrown with the first button, it will be thrown and logged for every single button - this is a massive performance hit: throwing and catching exceptions like this, is pretty expensive.

I'm ok with the dynamic cast - though some reflection might actually perform better, but without changing the logic to stop making that test for every single button it would still be inefficient.

The first HasPictureProperty return value will be the value returned by every single subsequent call: you need to change the logic so as to avoid making that check more than once.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.