https://devblogs.microsoft.com/oldnewthing/20240105-00/?p=109242 Skip to main content [RE1Mu3b] Microsoft The Old New Thing The Old New Thing The Old New Thing * Home * DevBlogs * Developer + Visual Studio + Visual Studio Code + Visual Studio for Mac + DevOps + Windows Developer + Developer support + ISE Developer + Engineering@Microsoft + Azure SDK + Command Line + Perf and Diagnostics + Notification Hubs + Math in Office + React Native * Technology + DirectX + PIX + Semantic Kernel + SurfaceDuo + Startups + Sustainable Engineering + Windows AI Platform * Languages + C++ + C# + F# + TypeScript + PowerShell Community + PowerShell Team + Python + Q# + JavaScript + Java + Java Blog in Chinese * .NET + All .NET posts + .NET MAUI + ASP.NET Core + Blazor + Entity Framework + ML.NET + NuGet + Servicing + Xamarin + .NET Blog in Chinese * Platform Development + #ifdef Windows + Azure Depth Platform + Azure Government + Azure VM Runtime Team + Bing Dev Center + Microsoft Edge Dev + Microsoft Azure + Microsoft 365 Developer + Microsoft Entra Identity Developer Blog + Old New Thing + Power Platform + Windows MIDI and Music dev * Data Development + Azure Cosmos DB + Azure Data Studio + Azure SQL Database + OData + Revolutions R + SQL Server Data Tools * More [ ] Search Search * No results Cancel Email Subscriptions are here! Get notified in your email when a new post is published to this blog Subscribe Close The case of the vector with an impossibly large size [png] Raymond Chen January 5th, 20244 1 A customer had a program that crashed with this stack: contoso!Widget::GetCost contoso!StandardWidgets::get_TotalCost+0x12f rpcrt4!Invoke+0x73 rpcrt4!Ndr64StubWorker+0xb9b rpcrt4!NdrStubCall3+0xd7 combase!CStdStubBuffer_Invoke+0xdb combase!ObjectMethodExceptionHandlingAction< >+0x47 combase!DefaultStubInvoke+0x376 combase!ServerCall::ContextInvoke+0x6f3 combase!ComInvokeWithLockAndIPID+0xacb combase!ThreadInvoke+0x103 rpcrt4!DispatchToStubInCNoAvrf+0x18 rpcrt4!RPC_INTERFACE::DispatchToStubWorker+0x1a9 rpcrt4!RPC_INTERFACE::DispatchToStubWithObject+0x1a7 rpcrt4!LRPC_SCALL::DispatchRequest+0x308 rpcrt4!LRPC_SCALL::HandleRequest+0xdcb rpcrt4!LRPC_SASSOCIATION::HandleRequest+0x2c3 rpcrt4!LRPC_ADDRESS::HandleRequest+0x183 rpcrt4!LRPC_ADDRESS::ProcessIO+0x939 rpcrt4!LrpcIoComplete+0xff ntdll!TppAlpcpExecuteCallback+0x14d ntdll!TppWorkerThread+0x4b4 kernel32!BaseThreadInitThunk+0x18 ntdll!RtlUserThreadStart+0x21 They wondered if some recent change to Windows was the source of the problem, since it didn't happen as much in earlier versions of Windows. The stack trace pointed to Widget::IsEnabled, which was crashing on the first instruction because it was given an invalid this pointer. 00007fff`73a8a59f mov edx,dword ptr [rcx+40h] ds:00000000`00000040=???????? The Widget pointer came from a std::vector that is a member of the StandardWidgets class. using namespace Microsoft::WRL; class StandardWidgets : RuntimeClass { IFACEMETHODIMP get_TotalCost(INT32* result); [ other methods not relevant here [?] private: HRESULT LazyInitializeWidgetList(); static constexpr PCWSTR standardWidgetNames[] = { L"Bob", L"Carol", L"Ted", L"Alice" }; static constexpr int standardWidgetCount = ARRAYSIZE(standardWidgetNames); std::vector> m_widgets; }; The code crashed at this call to Widget::GetCost: IFACEMETHODIMP StandardWidgets::get_TotalCost(INT32* result) { *result = 0; RETURN_IF_FAILED(LazyInitializeWidgetList()); INT32 totalCost = 0; for (int i = 0; i < standardWidgetCount; i++) { totalCost += m_widgets[i]->GetCost(); // here } *result = totalCost; return S_OK; } The customer's debugging showed that at the point of the crash, not only was the widget garbage, but the m_widgets vector had an impossibly large number of elements. The m_widgets is expected to have only four widgets, but it somehow found itself with ten, and sometimes as many as a hundred widgets. Of course, they were nearly all corrupted. Here's the code that lazy-initializes the widget list: HRESULT StandardWidgets::LazyInitializeWidgetList() { // Early-out if already initialized. if (!m_widgets.empty()) { return S_OK; } // Lazy-create the vector of standard widgets try { m_widgets.reserve(standardWidgetCount); for (auto name : standardWidgetNames) { ComPtr widget; RETURN_IF_FAILED( MakeAndInitialize(&widget, name)); m_widgets.push_back(widget); } } catch (std::bad_alloc const&) { return E_OUTOFMEMORY; } return S_OK; } The customer noted that the reserve method is always called with the value 4, and the code never pushes more than four items into the vector. They admitted that if there is a problem creating all four of the standard widgets, the vector could end up with fewer than four widgets, but it should never have more than four. You already have multiple clues that point toward what the customer's problem is. I'll give you some time to think about it. In the meantime, let's look at other issues with how the code lazy-initializes the widget list. As the customer noted, if there is a problem creating any of the four standard widgets, the failure is propagated to the caller of LazyInitializeWidgetList, and get_TotalCost in turn propagates the error to its own caller, and it never gets to the point where it walks through the vector adding up all the costs. If there is a memory allocation failure at the reserve(), or if there is a problem with the first standard widget, then the vector remains empty, and a second call to LazyInitializeWidgetList will make a new attempt at initialization. If there is a problem with the second or subsequent standard widget, however, things get weird. The LazyInitializeWidgetList function returns a failure, which causes get_TotalCost to return failure. But the second time someone calls get_TotalCost, LazyInitializeWidgetList will see a nonempty vector and assume that everything was initialized. This time, the get_TotalCost method will proceed with the summation and perform an out-of-bounds array access when it gets to the widget that failed to be created. Oops. This particular problem boils down to leaving a partially-initialized m_widgets if the lazy initialization fails. To avoid this problem, we should create the vector in a local variable and transfer it to the member variable only after we are sure all of the widgets were created successfully. HRESULT StandardWidgets::LazyInitializeWidgetList() { // Early-out if already initialized. if (!m_widgets.empty()) { return S_OK; } // Lazy-create the vector of standard widgets try { std::vector> widgets; widgets.reserve(standardWidgetCount); for (auto name : standardWidgetNames) { ComPtr widget; RETURN_IF_FAILED( MakeAndInitialize(&widget, name)); widgets.push_back(widget); } m_widgets.swap(widgets); } catch (std::bad_alloc const&) { return E_OUTOFMEMORY; } return S_OK; } This ensures that the m_widgets is either totally empty or totally initialized. It is never in a half-initialized state. While we're at it, we probably should convert the loop in get_TotalCost into a ranged for loop. Right now, get_TotalCost has a hidden dependency on LazyInitializeWidgets: It assumes that LazyInitializeWidgets always creates exactly the number of widgets as there are standardWidgetNames. Maybe in the future, you might want to suppress some of the standard widgets based on some configuration setting. If you add that configuration setting and forget to update get_TotalCost to account for suppressed widgets, you will have an out-of-bounds index. All the logic to decide which widgets are standard should be local to LazyInitializeWidgets. IFACEMETHODIMP StandardWidgets::get_TotalCost(INT32* result) { *result = 0; RETURN_IF_FAILED(LazyInitializeWidgetList()); INT32 totalCost = 0; for (auto&& widget : m_widgets) { totalCost += widget->GetCost(); } *result = totalCost; return S_OK; } Or if you want to get fancy, IFACEMETHODIMP StandardWidgets::get_TotalCost(INT32* result) { *result = 0; RETURN_IF_FAILED(LazyInitializeWidgetList()); *result = std::transform_reduce( m_widgets.begin(), m_widgets.end(), 0, std::plus<>(), [](auto&& w) { return w->GetCost(); }); return S_OK; } Okay, but back to the crash. I think I've added enough filler to give you time to consider what is happening. When I looked at this crash, I noticed that the class is implemented with the Microsoft::WRL::RuntimeClass template class, and the implementation explicitly listed FtmBase as a template parameter, marking this class as free-threaded (also known as "agile"), which means that it can be used from multiple threads simultaneously.1 The object is eligible for multithreaded use, but there are no mutexes to protect two threads from modifying m_widgets at the same time. I suspected a race condition. You can also observe that the class is being used in a free-threaded manner because the stack trace that leads to the crash says that it's running on a thread pool thread (TppWorkerThread), and thread pool threads default to the multi-threaded apartment.2 The only code on the stack between TppWorkerThread and the application code is all COM and RPC, so no application code snuck in and initialized the thread into single-threaded apartment mode. And when I looked at the crash dump, I caught the code red-handed: There was another thread also calling into this code. // Crashing thread 0:006> .frame 1 01 contoso!StandardWidgets::get_TotalCost+0x12f 0:006> dv this = 0x000001b7`492833b0 ... // Another thread running at the time of the crash 0:004> kn # Call Site 00 ntdll!ZwDelayExecution+0x14 01 ntdll!RtlDelayExecution+0x4c 02 KERNELBASE!SleepEx+0x84 04 kernel32!WerpReportFault+0xa4 05 KERNELBASE!UnhandledExceptionFilter+0xd3a02 06 ntdll!TppExceptionFilter+0x7a 07 ntdll!TppWorkerpInnerExceptionFilter+0x1a 08 ntdll!TppWorkerThread$filt$3+0x19 09 ntdll!__C_specific_handler+0x96 0a ntdll!__GSHandlerCheck_SEH+0x6a 0b ntdll!RtlpExecuteHandlerForException+0xf 0c ntdll!RtlDispatchException+0x2d4 0d ntdll!KiUserExceptionDispatch+0x2e 0e contoso!StandardWidgets::get_TotalCost+0x12f 0f rpcrt4!Invoke+0x73 10 rpcrt4!Ndr64StubWorker+0xb9b 11 rpcrt4!NdrStubCall3+0xd7 12 combase!CStdStubBuffer_Invoke+0xdb 14 combase!ObjectMethodExceptionHandlingAction< >+0x47 16 combase!DefaultStubInvoke+0x376 1a combase!ServerCall::ContextInvoke+0x6f3 1f combase!ComInvokeWithLockAndIPID+0xacb 21 combase!ThreadInvoke+0x103 22 rpcrt4!DispatchToStubInCNoAvrf+0x18 23 rpcrt4!RPC_INTERFACE::DispatchToStubWorker+0x1a9 25 rpcrt4!RPC_INTERFACE::DispatchToStubWithObject+0x1a7 27 rpcrt4!LRPC_SCALL::DispatchRequest+0x308 29 rpcrt4!LRPC_SCALL::HandleRequest+0xdcb 2a rpcrt4!LRPC_SASSOCIATION::HandleRequest+0x2c3 2b rpcrt4!LRPC_ADDRESS::HandleRequest+0x183 2c rpcrt4!LRPC_ADDRESS::ProcessIO+0x939 2d rpcrt4!LrpcIoComplete+0xff 2e ntdll!TppAlpcpExecuteCallback+0x14d 2f ntdll!TppWorkerThread+0x4b4 30 kernel32!BaseThreadInitThunk+0x18 31 ntdll!RtlUserThreadStart+0x21 0:004> .frame 0xe 0e contoso!StandardWidgets::get_TotalCost+0x12f 0:004> dv this = 0x000001b7`492833b0 ... Notice that the this pointer is the same for both threads, so we have proof that this object is being used from multiple threads simultaneously. The fix for the multithreading issue is to ensure that only one thread tries to initialize the widget vector at a time. We can do this by adding a mutex, but I'm going to go even further and use the std::once_flag, whose purpose in life is to be used in conjunction with std::call_once to perform thread-safe one-time initialization, which is exactly what we want. class StandardWidgets : RuntimeClass { IFACEMETHODIMP get_TotalCost(INT32* result); [ other methods not relevant here [?] private: HRESULT LazyInitializeWidgetList(); static constexpr PCWSTR standardWidgetNames[] = { L"Bob", L"Carol", L"Ted", L"Alice" }; static constexpr int standardWidgetCount = ARRAYSIZE(standardWidgetNames); std::once_flag m_initializeFlag; std::vector> m_widgets; }; HRESULT StandardWidgets::LazyInitializeWidgetList() { try { std::call_once(m_initializeFlag, [&] { std::vector> widgets; widgets.reserve(standardWidgetCount); for (auto name : standardWidgetNames) { ComPtr widget; THROW_IF_FAILED( MakeAndInitialize(&widget, name)); widgets.push_back(widget); } m_widgets = std::move(widgets); }); } catch (std::bad_alloc const&) { return E_OUTOFMEMORY; } return S_OK; } Update: We had to change the RETURN_IF_FAILED to THROW_IF_FAILED because (1) without the change, the compiler will complain that not all code paths return a value, because the lambda also falls off the end, and more importantly, (2) call_once doesn't care about the lambda return value; it uses exceptions to detect errors. End Update. This version also deals with the edge case where there are no standard widgets at all. The original code would continuously try to reinitialize the vector since it couldn't tell whether an empty vector means "Not yet initialized" or "Successfully initialized (and it's empty)". Bonus chatter: How did this multithreaded race condition lead to a vector with a ridiculous size? Well, we saw some time ago that the internal structure of a std::vector is three pointers, one for the start of the vector data, one for the end of the valid data, and one for the end of the allocated data. If two threads call reserve() simultaneously, both will allocate new data, and then they were race to update the three pointers. You might end up with a "start" pointer that points to the data allocated by the first thread, but an "end" pointer that points to the data allocated by the second thread, resulting in a vector of unusual size. 1 It was actually convenient that the implementation lists FtmBase explicitly, since the default behavior varies depending on whether __WRL_CONFIGURATION_LEGACY__ is set. +--------------------------------------------------------------+ | Template parameter | Standard mode | Legacy mode | |--------------------------+---------------+-------------------| | Nothing specified | Free-threaded | Not free-threaded | |--------------------------+-----------------------------------| | FtmBase | Free-threaded | |--------------------------+-----------------------------------| | InhibitFtmBase | Not free-threaded | |--------------------------+-----------------------------------| | InhibitFtmBase + FtmBase | Not free-threaded | +--------------------------------------------------------------+ In pseudocode: bool isFreeThreaded = !InhibitFtmBase && (FtmBase || Standard mode); The explicitly inclusion of FtmBase saved me the trouble of looking up what mode the customer's project is using. 2 Assuming the multi-threaded apartment exists at all. [png] Raymond Chen Follow Tagged Code Read next How do I prevent my C++/WinRT implementation class from participating in COM aggregation? Looking for a clue. [png]Raymond Chen January 8, 2024 0 comment In C++/WinRT, how can I await multiple coroutines and capture the results?, part 1 Using a custom awaiter to suppress the GetResults(). [png]Raymond Chen January 10, 2024 0 comment 4 comments Leave a commentCancel reply Log in to join the discussion. * [png] Bwmat January 5, 2024 11:14 am 1 collapse this comment copy link to this comment I believe you're 'losing' the error from the following line in your final solution (the return value from the lambda is ignored [and on success it falls off the end?]) RETURN_IF_FAILED( MakeAndInitialize(&widget, name)); Log in to Vote or Reply * [png] Ian Boyd January 6, 2024 5:13 pm 0 collapse this comment copy link to this comment I loved your old post about lazy initialization, constructing it on the stack, and using InterlockedCompareExchange to put it into the global/member variable (and destroy it if we lost the race). It's such an elegant and simple solution, and I now use it all the time. Log in to Vote or Reply * [png] Henke37 January 7, 2024 11:52 am 0 collapse this comment copy link to this comment When I see words like "it didn't happen as much" it tips me off that there are threading shenanigans at play. Log in to Vote or Reply * [png] Yukkuri Reimu January 10, 2024 6:12 pm 0 collapse this comment copy link to this comment "What about the VOUSes?" "Vectors of unusual size? I don't believe they exist." *Wham* Log in to Vote or Reply Archive * January 2024 * December 2023 * November 2023 * October 2023 * September 2023 * August 2023 * July 2023 * June 2023 * May 2023 * April 2023 * March 2023 * February 2023 * January 2023 * December 2022 * November 2022 * October 2022 * September 2022 * August 2022 * July 2022 * June 2022 * May 2022 * April 2022 * March 2022 * February 2022 * January 2022 * December 2021 * November 2021 * October 2021 * September 2021 * August 2021 * July 2021 * June 2021 * May 2021 * April 2021 * March 2021 * February 2021 * January 2021 * December 2020 * November 2020 * October 2020 * September 2020 * August 2020 * July 2020 * June 2020 * May 2020 * April 2020 * March 2020 * February 2020 * January 2020 * December 2019 * November 2019 * October 2019 * September 2019 * August 2019 * July 2019 * June 2019 * May 2019 * April 2019 * March 2019 * February 2019 * January 2019 * December 2018 * November 2018 * October 2018 * September 2018 * August 2018 * July 2018 * June 2018 * May 2018 * April 2018 * March 2018 * February 2018 * January 2018 * December 2017 * November 2017 * October 2017 * September 2017 * August 2017 * July 2017 * June 2017 * May 2017 * April 2017 * March 2017 * February 2017 * January 2017 * December 2016 * November 2016 * October 2016 * September 2016 * August 2016 * July 2016 * June 2016 * May 2016 * April 2016 * March 2016 * February 2016 * January 2016 * December 2015 * November 2015 * October 2015 * September 2015 * August 2015 * July 2015 * June 2015 * May 2015 * April 2015 * March 2015 * February 2015 * January 2015 * December 2014 * November 2014 * October 2014 * September 2014 * August 2014 * July 2014 * June 2014 * May 2014 * April 2014 * March 2014 * February 2014 * January 2014 * December 2013 * November 2013 * October 2013 * September 2013 * August 2013 * July 2013 * June 2013 * May 2013 * April 2013 * March 2013 * February 2013 * January 2013 * December 2012 * November 2012 * October 2012 * September 2012 * August 2012 * July 2012 * June 2012 * May 2012 * April 2012 * March 2012 * February 2012 * January 2012 * December 2011 * November 2011 * October 2011 * September 2011 * August 2011 * July 2011 * June 2011 * May 2011 * April 2011 * March 2011 * February 2011 * January 2011 * December 2010 * November 2010 * October 2010 * September 2010 * August 2010 * July 2010 * June 2010 * May 2010 * April 2010 * March 2010 * February 2010 * January 2010 * December 2009 * November 2009 * October 2009 * September 2009 * August 2009 * July 2009 * June 2009 * May 2009 * April 2009 * March 2009 * February 2009 * January 2009 * December 2008 * November 2008 * October 2008 * September 2008 * August 2008 * July 2008 * June 2008 * May 2008 * April 2008 * March 2008 * February 2008 * January 2008 * December 2007 * November 2007 * October 2007 * September 2007 * August 2007 * July 2007 * June 2007 * May 2007 * April 2007 * March 2007 * February 2007 * January 2007 * December 2006 * November 2006 * October 2006 * September 2006 * August 2006 * July 2006 * June 2006 * May 2006 * April 2006 * March 2006 * February 2006 * January 2006 * December 2005 * November 2005 * October 2005 * September 2005 * August 2005 * July 2005 * June 2005 * May 2005 * April 2005 * March 2005 * February 2005 * January 2005 * December 2004 * November 2004 * October 2004 * September 2004 * August 2004 * July 2004 * June 2004 * May 2004 * April 2004 * March 2004 * February 2004 * January 2004 * December 2003 * November 2003 * October 2003 * September 2003 * August 2003 * July 2003 Relevant Links I wrote a book Ground rules Disclaimers and such My necktie's Twitter Categories Code History Tips/Support Other Non-Computer Stay informed [ ] [Subscribe] By subscribing you agree to our Terms of Use and Privacy Policy Share on Social media * * * Login Theme * light-theme-iconLight * dark-theme-iconDark Insert/edit link Close Enter the destination URL URL [ ] Link Text [ ] [ ] Open link in a new tab Or link to existing content Search [ ] No search term specified. Showing recent items. Search or use up and down arrow keys to select an item. Cancel [Add Link] Code Block x Paste your code snippet [ ] Cancel Ok Feedback usabilla icon What's new * Surface Laptop Studio 2 * Surface Laptop Go 3 * Surface Pro 9 * Surface Laptop 5 * Surface Studio 2+ * Copilot in Windows * Microsoft 365 * Windows 11 apps Microsoft Store * Account profile * Download Center * Microsoft Store support * Returns * Order tracking * Certified Refurbished * Microsoft Store Promise * Flexible Payments Education * Microsoft in education * Devices for education * Microsoft Teams for Education * Microsoft 365 Education * How to buy for your school * Educator training and development * Deals for students and parents * Azure for students Business * Microsoft Cloud * Microsoft Security * Dynamics 365 * Microsoft 365 * Microsoft Power Platform * Microsoft Teams * Microsoft Industry * Small Business Developer & IT * Azure * Developer Center * Documentation * Microsoft Learn * Microsoft Tech Community * Azure Marketplace * AppSource * Visual Studio Company * Careers * About Microsoft * Company news * Privacy at Microsoft * Investors * Diversity and inclusion * Accessibility * Sustainability Your Privacy Choices Your Privacy Choices * Sitemap * Contact Microsoft * Privacy * Manage cookies * Terms of use * Trademarks * Safety & eco * Recycling * About our ads * (c) Microsoft 2023