Skip to content

Conversation

ggcrunchy
Copy link
Contributor

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 added 2 commits July 1, 2022 12:17
…uilt class (with single-threaded settings, to match the original call) instead

Made notes of DirectX and alcConfig non-leaks

Made note of stubborn leak in Rtt_Allocator (heh)

Actual leaks fixed: audio session manager was not closed (minor), nor was analytics Lua context (not exactly major, but quite a few allocations)
@ggcrunchy
Copy link
Contributor Author

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.

@Shchvova Shchvova changed the base branch from master to experimental January 27, 2023 04:52
@Shchvova Shchvova self-requested a review as a code owner January 27, 2023 04:52
@Shchvova Shchvova merged commit 7c236d4 into coronalabs:experimental Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants