corona icon indicating copy to clipboard operation
corona copied to clipboard

COM balancing and some fixed leaks

Open ggcrunchy opened this issue 3 years ago • 1 comments

I've been doing some hunting in Debug mode and often when closing a session would trigger an assert in wincore.cpp, line 1055. I dug into the MFC code and it led me to this post: https://microsoft.public.vc.mfc.narkive.com/cGo3iW5V/afxmaphwnd-causing-an-assert-why.

Following the comments, I did a little further investigation and found that the CoInitialize(nullptr) done by the Windows simulator view is never answered by a CoUninitialize(). This is supposedly a source of leaks and such.

I had also seen there was a class, ScopedComInitializer, intended to address this, so I made that a member of CSimulatorView and removed the original call. Per the CoInitialize docs, I used the single-threaded apartment type.

The assert seems to have gone away when quitting normally. I think exiting while stepping through code might still be able to do it—there is audio code without benefit of the scoped initializer—however, this is a marked improvement. 😄


Also in Debug mode, when quitting via the window's "X" (in Windows), you also get a memory leak report.

I finally decided to track these all down. (This was with some simple applications; maybe something more significant would flush out others.)

A few of these are valid. The audio session manager was never closed (just a pointer, 4 bytes), nor was the simulator analytics Lua context (small allocations, but quite a few of them).

Oddly enough, Rtt_Allocator has defeated my efforts to fix. 😃This is reference-counted, but the create / destroy calls are not yet balanced, so it leaks a pointer (4 bytes, again).

There are some further "leaks" in audio code: a std::unordered_set of thread IDs and some AL config info. These are false positives, though. The memory report is issued before the DLL cleanup that does in fact unload them.

I've made notes in the allocator and those two audio bits.


For reference, you should now see something like this in a report:

{279} normal block at 0x005E4748, 4 bytes long.
 Data: <    > 00 00 00 00 
{180} normal block at 0x005F8B58, 64 bytes long.
 Data: <8 _ 8 _ 8 _ 8 _ > 38 90 5F 00 38 90 5F 00 38 90 5F 00 38 90 5F 00 
{179} normal block at 0x005F91C0, 8 bytes long.
 Data: <( !z    > 28 EC 21 7A 00 00 00 00 
{178} normal block at 0x005F9620, 8 bytes long.
 Data: <  !z    > 1C EC 21 7A 00 00 00 00 
{177} normal block at 0x005F9038, 12 bytes long.
 Data: <8 _ 8 _     > 38 90 5F 00 38 90 5F 00 CD CD CD CD 
{96} normal block at 0x005E2B98, 8 bytes long.
 Data: <general > 67 65 6E 65 72 61 6C 00 
{95} normal block at 0x005E2BD0, 12 bytes long.
 Data: < +^         > 98 2B 5E 00 00 00 00 00 00 00 00 00 

On this run, allocation 279 was the Rtt_Allocator; 177 - 180 were the std::unordered_set; and 95 - 96 were the "first" AL ConfigBlock and its name ("general").

ggcrunchy avatar Jul 01 '22 18:07 ggcrunchy

This will leak if the params are invalid. It also looks like the same resources get lost if the request isn't completed, but I haven't tested.

This was prompted by the title of an otherwise meandering forum post ("Is there a leak in network.download?") from the moderation queue; looks like that got rejected.

Maybe I'll make a round through the samples and see if anything else gets flushed out.

ggcrunchy avatar Jul 12 '22 19:07 ggcrunchy