MicrosoftDocs / windows-dev-docs

Conceptual and overview content for developing Windows apps
Creative Commons Attribution 4.0 International
667 stars 1.18k forks source link

Wrapping collections seem unstable #1382

Closed sichbo closed 5 years ago

sichbo commented 5 years ago

I may doing something wrong here, but a basic custom collection returned by a WinRT component seems to crash the IDE debugger.

I've posted an MVCE here: https://github.com/sichbo/WinRTCollectionCrash

The gist of it is: Basic MIDL definition

namespace A {
    [default_interface]
    runtimeclass ThingCollection : Windows.Foundation.Collections.IVector<Thing> {
        ThingCollection();
    }
    [default_interface]
    runtimeclass Thing : Windows.Foundation.IStringable {
        Thing();
        String Name;
        static ThingCollection GetThings();
    }
}

In your ThingCollection implementation, simply shim all of the interface methods into a backing:

    struct ThingCollection : ThingCollectionT<ThingCollection>
    {
        ThingCollection() = default;

        Windows::Foundation::Collections::IIterator<A::Thing> First();
        A::Thing GetAt(uint32_t index);
        uint32_t Size();
        Windows::Foundation::Collections::IVectorView<A::Thing> GetView();
        bool IndexOf(A::Thing const& value, uint32_t& index);
        void SetAt(uint32_t index, A::Thing const& value);
        void InsertAt(uint32_t index, A::Thing const& value);
        void RemoveAt(uint32_t index);
        void Append(A::Thing const& value);
        void RemoveAtEnd();
        void Clear();
        uint32_t GetMany(uint32_t startIndex, array_view<A::Thing> items);
        void ReplaceAll(array_view<A::Thing const> items);

    Windows::Foundation::Collections::IVector<A::Thing> m_Impl{ winrt::single_threaded_vector<A::Thing>() }; // use this for everything.

    };

Breakpoint on something like ThingCollection.Size() and have an external app consume the component. Hovering over m_Impl or attempting to see this in the watch window will hose the app and crash the debugging experience.

My assumption of the WinRT wizardry is that everything should be kept alive until the client app destroys its projected instance? What we're seeing instead are symptoms of stack imbalance/memory corruption. Digging into the disassembly we can see "this" is wiped out with a debugger hint address 0xCCCCCCCC. Trying to disassemble further up the ABI chain inside of the Size() method will crash the debugger.

Interestingly, you can get slightly further along by changing that IVector backing instance to const& like so:

...
Windows::Foundation::Collections::IVector<A::Thing> const& m_Impl{ winrt::single_threaded_vector<A::Thing>() };
...

… with that, you at least get what looks like a sane "this" pointer.. but alas, expanding it then crashes not only the debugger, but the full Visual Studio IDE.

What gives?


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

stevewhims commented 5 years ago

Hi, Simon.

I can repro the crash (I updated Microsoft.Windows.CppWinRT to the latest version in order to get a build). Then, I updated the C# project to 17763, and that got me further in the debugger. But still, there seemed to be an issue with the code someplace. I see you're using one of the helper functions for collections. In this case, though, I tweaked your code to use the equivalent one of the base classes for collections. And I was able to bypass the issue of m_Impl being invalid. FYI, here's the class:

    struct ThingCollection :
        implements<ThingCollection, IVector<A::Thing>, IVectorView<A::Thing>, IIterable<A::Thing>>,
        winrt::vector_base<ThingCollection, A::Thing>
    {
        ThingCollection()
        {
            m_values.push_back(winrt::make<A::implementation::Thing>());
        }

        auto& get_container() const noexcept
        {
            return m_values;
        }

        auto& get_container() noexcept
        {
            return m_values;
        }

    private:
        std::vector<A::Thing> m_values{ winrt::make<A::implementation::Thing>(), winrt::make<A::implementation::Thing>() };
    };

The C# project is Output-ing the expected Size(). Since that sidesteps the immediate issue, you might want to just go that way.

Meanwhile, I'll let the product team know about this GitHub Issue in case they want to advise us and/or file a bug on the debugger. And, just FYI, customers always have these options: For issues, or suspected product bugs, there's Microsoft Support; a support engineer will look at the repro and see whether the issue is a product bug or something else. For Visual Studio issues, there's the Visual Studio User Voice feedback website, or the Developer Community. The Windows developer feedback site is the right place to file a product bug.

For problems/errors/omissions with the actual documentation, this is the right forum to raise those. Please let me know if there's anything that I can do to improve the documentation here. And thanks again for the feedback and your diligence in reporting these issues. :) ~Steve

stevewhims commented 5 years ago

So, there is a known mixed-mode debugging bug in Visual Studio; the visualizer within the C++/WinRT VSIX contains a workaround. That combined with what I found, means that some combination of upgrading your VSIX and/or the target SDK of your projects should alleviate that issue. But if the base classes for collections work for you (you don't have to shim all those methods), then that might be better. ~Steve

sichbo commented 5 years ago

Thanks for that, Steve!

You're earning your coffee today :)

Many thanks for the vector_base sample and passing along the debugger issue and finding out about the known mixed-mode bug. Traditionally when I've encountered behaviour like this in VS it was my own stupid fault and needed Intel xe memory inspector to find the source (typically hand coded asm/intrinsic mistakes.)

My understanding of the wpdev uservoice site was that it was only for feature suggestions. If it's a good way to get bugs looked at like you say, I'll try filing a broker infrastructure bug that I've been sitting on. When I tried getting somebody to look at it through the msdn support forum, they just said I had to open an incident through the main support channel you linked. The problem with that is it's a $300 "gamble" as to whether or not it's actually a bug and you need to buy an incident prior to submission and I'm pretty sure there are no refunds either way. For an indie developer who hasn't earned a penny in three years that's a bit rough.

Anyhoo, it sounds like this issue is well in-hand already so we can close it out. Thanks again, Steve.

stevewhims commented 5 years ago

You're right, Simon, the Developer Community is probably better for bug reports than the UserVoice is. I just wanted to enumerate all the channels, as they're all ways to get data to Microsoft (some better channels than others for that), where "data" is "hey, someone else is having this issue, too." As a customer, I might feel like my single lone voice won't prevail, so why bother; on the other hand, until Microsoft gets a good sample of the population of folks an issue is affecting, we're not sure how to prioritize it. So it's great to get all the data we can. I can't speak with certainty to the refund question, but I'll make enquiries since it seems like something I should know for sure. :)

sichbo commented 5 years ago

Thanks again for the outstanding support, Steve. There was a "bugs" category on uservoice, so I went ahead and uploaded a MVCE on that other issue. Cheers!

stevewhims commented 5 years ago

Cool! You're welcome! :)

stevewhims commented 5 years ago

Just one last bit of info, Simon, FYI. You needn't be concerned on this score: "it's a $300 "gamble" as to whether or not it's actually a bug and you need to buy an incident prior to submission and I'm pretty sure there are no refunds either way."

For future reference: if you pay to open up a new professional support incident and the issue leads to a confirmed bug by the dev team, then you can request a refund and it will get processed. Just be sure to verify with the case owner that, when the case is closed, it's linked to the correct BUG number.

sichbo commented 5 years ago

Great to know — thanks for the tip!

Scottj1s commented 5 years ago

Note: Visual Studio 16.0.0 preview 3 will contain a fix for the mixed mode debugging issue mentioned by @stevewhims (the workaround was only partly effective)