[Bug 36272] New: valgrind shows several leaks in dmime/tests/performance.c

https://bugs.winehq.org/show_bug.cgi?id=36272

Bug ID: 36272
           Summary: valgrind shows several leaks in
                    dmime/tests/performance.c
           Product: Wine
           Version: 1.7.18
          Hardware: x86
                OS: Linux
            Status: NEW
          Keywords: download, source, testcase
          Severity: normal
          Priority: P2
         Component: directx-dmusic
          Assignee: [e-mail]
          Reporter: [e-mail]

==6437== 28 bytes in 1 blocks are definitely lost in loss record 301 of 951
==6437==    at 0x7BC4C735: notify_alloc (heap.c:255)
==6437==    by 0x7BC50F79: RtlAllocateHeap (heap.c:1716)
==6437==    by 0x7FBF4B1: DMUSIC_CreateDirectMusicImpl (dmusic.c:414)
==6437==    by 0x7FBFABB: ClassFactory_CreateInstance (dmusic_main.c:100)
==6437==    by 0x4D3AE26: CoCreateInstance (unknwn.h:226)
==6437==    by 0x5F9ADA9: IDirectMusicPerformance8Impl_Init (performance.c:286)
==6437==    by 0x4956EA6: test_createport (performance.c:104)
==6437==    by 0x4957D93: func_performance (performance.c:263)
==6437==    by 0x4958B90: run_test (test.h:584)
==6437==    by 0x4958F7F: main (test.h:654)
==6437== 
==6437== 44 bytes in 1 blocks are definitely lost in loss record 432 of 951
==6437==    at 0x7BC4C735: notify_alloc (heap.c:255)
==6437==    by 0x7BC50F79: RtlAllocateHeap (heap.c:1716)
==6437==    by 0x5F90B1E: create_dmaudiopath (audiopath.c:642)
==6437==    by 0x5F9D9FE: IDirectMusicPerformance8Impl_CreateStandardAudioPath
(performance.c:1033)
==6437==    by 0x5F9D5BF: IDirectMusicPerformance8Impl_InitAudio
(performance.c:952)
==6437==    by 0x4956AD4: test_InitAudio (performance.c:51)
==6437==    by 0x4957D53: func_performance (performance.c:256)
==6437==    by 0x4958B90: run_test (test.h:584)
==6437==    by 0x4958F7F: main (test.h:654)
==6437== 
==6437== 56 bytes in 1 blocks are possibly lost in loss record 480 of 951
==6437==    at 0x7BC4C735: notify_alloc (heap.c:255)
==6437==    by 0x7BC50F79: RtlAllocateHeap (heap.c:1716)
==6437==    by 0xC0632EC: MMDRV_InitPerType (lolvldrv.c:384)
==6437==    by 0xC063C27: MMDRV_Install (lolvldrv.c:475)
==6437==    by 0xC06416F: MMDRV_Init (lolvldrv.c:547)
==6437==    by 0xC061F4B: MMDRV_InitSingleType (lolvldrv.c:78)
==6437==    by 0xC061FEE: MMDRV_GetNum (lolvldrv.c:89)
==6437==    by 0xC07F7C8: midiOutGetNumDevs (winmm.c:284)
==6437==    by 0x7FBF004: create_system_ports_list (dmusic.c:328)
==6437==    by 0x7FBF5C7: DMUSIC_CreateDirectMusicImpl (dmusic.c:436)
==6437==    by 0x7FBFABB: ClassFactory_CreateInstance (dmusic_main.c:100)
==6437==    by 0x4D3AE26: CoCreateInstance (unknwn.h:226)
==6437==    by 0x5F9ADA9: IDirectMusicPerformance8Impl_Init (performance.c:286)
==6437==    by 0x5F9D498: IDirectMusicPerformance8Impl_InitAudio
(performance.c:930)
==6437==    by 0x4956AD4: test_InitAudio (performance.c:51)
==6437==    by 0x4957D53: func_performance (performance.c:256)
==6437==    by 0x4958B90: run_test (test.h:584)
==6437==    by 0x4958F7F: main (test.h:654)
==6437== 
==6437== 56 bytes in 1 blocks are possibly lost in loss record 481 of 951
==6437==    at 0x7BC4C735: notify_alloc (heap.c:255)
==6437==    by 0x7BC50F79: RtlAllocateHeap (heap.c:1716)
==6437==    by 0xC0632EC: MMDRV_InitPerType (lolvldrv.c:384)
==6437==    by 0xC063C4D: MMDRV_Install (lolvldrv.c:476)
==6437==    by 0xC06416F: MMDRV_Init (lolvldrv.c:547)
==6437==    by 0xC061F4B: MMDRV_InitSingleType (lolvldrv.c:78)
==6437==    by 0xC061FEE: MMDRV_GetNum (lolvldrv.c:89)
==6437==    by 0xC07F7C8: midiOutGetNumDevs (winmm.c:284)
==6437==    by 0x7FBF004: create_system_ports_list (dmusic.c:328)
==6437==    by 0x7FBF5C7: DMUSIC_CreateDirectMusicImpl (dmusic.c:436)
==6437==    by 0x7FBFABB: ClassFactory_CreateInstance (dmusic_main.c:100)
==6437==    by 0x4D3AE26: CoCreateInstance (unknwn.h:226)
==6437==    by 0x5F9ADA9: IDirectMusicPerformance8Impl_Init (performance.c:286)
==6437==    by 0x5F9D498: IDirectMusicPerformance8Impl_InitAudio
(performance.c:930)
==6437==    by 0x4956AD4: test_InitAudio (performance.c:51)
==6437==    by 0x4957D53: func_performance (performance.c:256)
==6437==    by 0x4958B90: run_test (test.h:584)
==6437==    by 0x4958F7F: main (test.h:654)
==6437== 
==6437== 552 bytes in 1 blocks are possibly lost in loss record 789 of 951
==6437==    at 0x7BC4C735: notify_alloc (heap.c:255)
==6437==    by 0x7BC50F79: RtlAllocateHeap (heap.c:1716)
==6437==    by 0x5F9E05B: create_dmperformance (performance.c:1218)
==6437==    by 0x5F91088: ClassFactory_CreateInstance (dmime_main.c:100)
==6437==    by 0x4D3AE26: CoCreateInstance (unknwn.h:226)
==6437==    by 0x4957997: test_COM (performance.c:218)
==6437==    by 0x4957D8E: func_performance (performance.c:262)
==6437==    by 0x4958B90: run_test (test.h:584)
==6437==    by 0x4958F7F: main (test.h:654)
==6437==

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

https://bugs.winehq.org/show_bug.cgi?id=36272

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |valgrind

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #1 from Austin English  ---
Another one:
LSA lib dlmisc.c:254:(snd1_dlobj_cache_get) Cannot open shared library
/usr/lib/alsa-lib/libasound_module_pcm_pulse.so
ALSA lib dlmisc.c:254:(snd1_dlobj_cache_get) Cannot open shared library
/usr/lib/alsa-lib/libasound_module_pcm_pulse.so
ALSA lib pcm.c:7905:(snd_pcm_recover) underrun occurred
ALSA lib pcm.c:7905:(snd_pcm_recover) underrun occurred
ALSA lib pcm.c:7905:(snd_pcm_recover) underrun occurred
==29211== 40 bytes in 1 blocks are definitely lost in loss record 314 of 778
==29211==    at 0x7BC4A9F1: notify_alloc (heap.c:254)
==29211==    by 0x7BC4EC94: RtlAllocateHeap (heap.c:1715)
==29211==    by 0x5CA35A5: ???
==29211==    by 0x5CAF3D6: ???
==29211==    by 0x5CAEE96: ???
==29211==    by 0x479C31D: test_InitAudio (performance.c:51)
==29211==    by 0x479D442: func_performance (performance.c:256)
==29211==    by 0x479E1F3: run_test (test.h:584)
==29211==    by 0x479E63B: main (test.h:666)
==29211==
{
   <insert_a_suppression_name_here>
   Memcheck:Leak
   match-leak-kinds: definite
   fun:notify_alloc
   fun:RtlAllocateHeap
   obj:*
   obj:*
   obj:*
   fun:test_InitAudio
   fun:func_performance
   fun:run_test
   fun:main
}

wine-1.7.44-149-g0922865

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #2 from Mathew Hodson  ---
Can this be closed?

Commit
https://source.winehq.org/git/wine.git/commitdiff/fa66c1b3011a32036b42c43632...
references this bug.

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

Michael Stefaniuc  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #3 from Michael Stefaniuc  ---
Not fixed yet as one of the initial leaks is still there:

==8759== 28 bytes in 1 blocks are definitely lost in loss record 241 of 915
==8759==    at 0x7BC71D2B: RtlAllocateHeap (heap.c:260)
==8759==    by 0x6C2F26E: MMDRV_InitPerType (lolvldrv.c:380)
==8759==    by 0x6C2F591: MMDRV_Install (lolvldrv.c:473)
==8759==    by 0x6C2F971: MMDRV_Init (lolvldrv.c:548)
==8759==    by 0x6C2FB1F: MMDRV_InitSingleType (lolvldrv.c:74)
==8759==    by 0x6C2FB1F: MMDRV_GetNum (???:0)
==8759==    by 0x6C46987: midiOutGetNumDevs (winmm.c:291)
==8759==    by 0x6BF8C95: DMUSIC_CreateDirectMusicImpl (dmusic.c:518)
==8759==    by 0x6BF945E: ClassFactory_CreateInstance (dmusic_main.c:99)
==8759==    by 0x5806A05: IClassFactory_CreateInstance (unknwn.h:223)
==8759==    by 0x5806A05: CoCreateInstanceEx (???:0)
==8759==    by 0x5806CCC: CoCreateInstance (compobj.c:3376)
==8759==    by 0x6BBB589: IDirectMusicPerformance8Impl_InitAudio
(performance.c:922)
==8759==    by 0x4DCA5A5: test_InitAudio (performance.c:97)
==8759==    by 0x4DCA5A5: func_performance (???:0)
==8759==    by 0x4DC413E: main (test.h:504)

This is from a valgrind run that Sven run for me for wine-5.0-rc4

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #4 from Michael Stefaniuc  ---
Actually the leak from comment #3 is in the initialization of winmm.dll which
gets de-initialized on dll unload. As the initialization happens only once the
"leak" is not a problem and thus can be safely ignored.

Also the rest of the entries are "possibly lost" and all but one have the same
pattern:
  test_InitAudio (performance.c:97)
  DMUSIC_CreateDirectMusicImpl (dmusic.c:518)
  midiOutGetNumDevs (winmm.c:291) (lolvldrv.c:508)
  MMDRV_Init (lolvldrv.c:508)

MMDRV_Init() leaks are probably a good candidate for a generic suppression.

For the remaining "possibly lost" I have submitted the patch:
https://source.winehq.org/patches/data/176406

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #5 from Michael Stefaniuc  ---
Patch for the last non MMDRV_Init() based leak got committed:
https://source.winehq.org/git/wine.git/?a=commit;h=5b96ed0207e5a871682bf8ff5...

But I'll let Austin validate my rationale and valgrind before resolving this
bug.

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #6 from Sven  ---
That leak is gone from my log now indeed.

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #7 from Austin English  ---
(In reply to Michael Stefaniuc from comment #5)
> Patch for the last non MMDRV_Init() based leak got committed:
> https://source.winehq.org/git/wine.git/?a=commit;
> h=5b96ed0207e5a871682bf8ff53635fd655cfdfcc
>
> But I'll let Austin validate my rationale and valgrind before resolving this
> bug.

I still see some after blacklisting MMDRV_Init() (wine-5.4-205-g3ddf3a720f):

==13437== 460 bytes in 1 blocks are definitely lost in loss record 885 of 1,120
==13437==    at 0x7BC6258C: notify_alloc (heap.c:260)
==13437==    by 0x7BC653DA: RtlAllocateHeap (heap.c:1716)
==13437==    by 0x7165973: synth_port_create (port.c:826)
==13437==    by 0x7160ECD: IDirectMusic8Impl_CreatePort (dmusic.c:280)
==13437==    by 0x7120FAA: ???
==13437==    by 0x7122BE6: ???
==13437==    by 0x4EE6924: test_pchannel (performance.c:355)
==13437==    by 0x4EE87AC: func_performance (performance.c:656)
==13437==    by 0x4EE8ADF: run_test (test.h:560)
==13437==    by 0x4EE919E: main (test.h:648)
==13437==

==13437== 864 bytes in 1 blocks are definitely lost in loss record 996 of 1,120
==13437==    at 0x7BC6258C: notify_alloc (heap.c:260)
==13437==    by 0x7BC653DA: RtlAllocateHeap (heap.c:1716)
==13437==    by 0x1B7A346A: ???
==13437==    by 0x4EE829C: test_notification_type (performance.c:542)
==13437==    by 0x4EE87B1: func_performance (performance.c:657)
==13437==    by 0x4EE8ADF: run_test (test.h:560)
==13437==    by 0x4EE919E: main (test.h:648)
==13437==

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

François Gouget  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #8 from François Gouget  ---
Are the two leaks from comment 7 confirmed / still present? If I read this bug
correctly it seems like all the others should be fixed.

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

David Kahurani  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #9 from David Kahurani  ---
Yes, I believe these two leaks are still present. While I expect that both of
the tests free all resources when the test is done, this is not the case. The
memory footprint of the test module increases steadily when either of these two
tests is made to run repeatedly which indicates to me that the leak(s) are
still present.

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #10 from David Kahurani  ---
Actually, it looks like there are more than just these two leaks.

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #11 from Michael Stefaniuc  ---
I do not see these leaks in valgrind. I'm attaching the output of
export VALGRIND_OPTS="--trace-children=yes --track-origins=yes
--gen-suppressions=all
--suppressions=$HOME/wine/austin987/valgrind/valgrind-suppressions-ignore
--suppressions=$HOME/wine/austin987/valgrind/valgrind-suppressions-external
--num-callers=20 --vex-iropt-register-updates=allregs-at-mem-access
--leak-check=full --show-leak-kinds=all"
WINETEST_WRAPPER=valgrind make dlls/dmime/tests/i386-windows/performance.ok

Though I had to add an additional suppression to valgrind-suppressions-ignore
else it would hit 10 Million errors and stop reporting / counting:
{
   wine_syscall_dispatcher
   Memcheck:Addr2
   fun:__wine_syscall_dispatcher
   obj:*
}

Also I had to kill the dmime_test.exe at the end as it was sitting in an
infinite loop (100% CPU, strace showing just a blocking read()). Or seldom it
would be crashing.

But I'm not seeing those definitely lost loss records.

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #12 from Michael Stefaniuc  ---
Created attachment 73716
  --> https://bugs.winehq.org/attachment.cgi?id=73716
valgrind log with --leak-check=full --show-leak-kinds=all

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

Zeb Figura  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #13 from Zeb Figura  ---
I don't think the ntdll heap code is hooked up anymore.

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #14 from David Kahurani  ---
(In reply to Michael Stefaniuc from comment #11)
> But I'm not seeing those definitely lost loss records.

I tracked one of the leaks (the first one in comment #7) to reference leaks on
the  IDirectMusicPort object. The methods marked 'todo' do increment the
reference count on the object and also, we should probably call
IDirectMusicPerformance8_RemovePort when we are done with object. I'm not sure
what to do about the tests marked 'todo'

To observe this, you can run the tests with WINEDEDUG=+dmime,+dmusic, the
IDirectMusicPort object never gets released because the reference count is a
high number, like 15 or something

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #15 from David Kahurani  ---
Created attachment 73723
  --> https://bugs.winehq.org/attachment.cgi?id=73723
Proposed patch

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #16 from David Kahurani  ---
PChannelInfo not only fail but also leaks a reference which I guess we have to
remove manually.

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #17 from Michael Stefaniuc  ---
Yeah, I was more looking for a working valgrind setup than for the actual
leaks...

There's some oddity on how the ports are mapped on pchannels aka not
refcounted.
Needs more tests of course to see how native handles that. But basically the
code has issue more so than the tests.

Test leaks could be worked around at the end with a
    while(IDirectMusicPort_Release(portX) > 0);
But that just hides the issue.

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #18 from David Kahurani  ---
(In reply to David Kahurani from comment #10)
> Actually, it looks like there are more than just these two leaks.

I didn't initially read the whole thread, but, it looks like there is a known
memory leak in this code as indicated in comment #4. This is probably the extra
leak I am experiencing... or so I think.

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #19 from David Kahurani  ---
(In reply to Michael Stefaniuc from comment #17)
> Yeah, I was more looking for a working valgrind setup than for the actual
> leaks...

If I am not mistaken, there's been an on-going migration; migrating from Heap*
windows functions to malloc/free/realloc. I don't know for sure why this
migration has been going on but it looks like it could help in here.

If you are okay with this, I could migrate the modules...

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #20 from Michael Stefaniuc  ---
The migration malloc / free won't fix the leaks.
The migrations are happening now only because we can do them with the
conversion to PE as we can use the malloc and free from the msvcrt DLL.
It avoids the need for the heap_alloc / heap_free wrappers; basically less
typing for the developers. And provides a sane behaviour for realloc which
HeapRealloc lacks.

Also the migrations aren't a pure search and replace either. Some stuff needs
to stay with HeapAlloc aka those public API parts e.g. seen in the
documentation as "the caller is responsible to HeapFree() the returned object".

[Bug 36272] valgrind shows several leaks in dmime/tests/performance.c

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=36272

--- Comment #21 from David Kahurani  ---
(In reply to Michael Stefaniuc from comment #20)
> The migration malloc / free won't fix the leaks.

I was under the impression that the migration uses system malloc/free/realloc
which would allow us to setup and use Valgrind again.