http://bugs.winehq.org/show_bug.cgi?id=28023
Summary: CoreAudio queue memory leak Product: Wine Version: 1.3.25 Platform: x86 OS/Version: Mac OS X 10.5 Status: UNCONFIRMED Severity: normal Priority: P2 Component: mmdevapi AssignedTo: wine-bugs@winehq.org ReportedBy: hoehle@users.sourceforge.net CC: aeikum@codeweavers.com
While working on a patch to fix the Get/ReleaseBuffer ordering, see bug #27184 comment #3, I found a bug in memory management in winecoreaudio's mmdevapi.
510 HeapFree(GetProcessHeap(), 0, This->public_buffer); but public_buffer originates from 1625 sc = AudioQueueAllocateBuffer(This->aqueue, bytes, 1626 &This->public_buffer); The APIs don't match. It currently doesn't crash because public_buffer is NULL upon regular exit. This matches: 1634 AudioQueueFreeBuffer(This->aqueue, This->public_buffer);
That would be easy to fix, but it leads to question how AudioClient_Release frees the buffer queue.
502 if(This->aqueue) 503 AudioQueueDispose(This->aqueue, 1);
I suppose that Dispose gets rid of all the MacOS-level elements left in the queue, but I doubt it can handle the additional AQBuffer that Wine links to every object via the mUserData slot.
1632 buf = HeapAlloc(GetProcessHeap(), 0, sizeof(AQBuffer)); 1640 This->public_buffer->mUserData = buf;
Therefore I believe Wine must iterate through the list (aqueue or avail_buffers?) and free remaining objects one by one.
As I'm not familiar with the MacOS API's, I'd like somebody knowledgeable (Andrew, Ken?), to check this issue and write the patch.
BTW, I believe that correct freeing should first return This->public_buffer into the queue -- where it originated -- like I did in my ordering patch, then delete the whole queue.
+ if(This->public_buffer){ + AQBuffer *buf = This->public_buffer->mUserData; + list_add_tail(&This->avail_buffers, &buf->entry); + This->public_buffer = NULL; + }
http://bugs.winehq.org/show_bug.cgi?id=28023
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- OS/Version|Mac OS X 10.5 |Mac OS X
http://bugs.winehq.org/show_bug.cgi?id=28023
--- Comment #1 from Jörg Höhle hoehle@users.sourceforge.net 2011-08-25 02:59:57 CDT --- I'm working on this one. The leak is plugged, but it needs to be augmented with yet another patch about robust buffer handling when some of the OS calls fail during capture that I've not finished yet.
http://bugs.winehq.org/show_bug.cgi?id=28023
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |1.4.0
--- Comment #2 from Austin English austinenglish@gmail.com 2011-10-09 15:24:01 CDT --- Sound bug => 1.4 milestone.
http://bugs.winehq.org/show_bug.cgi?id=28023
--- Comment #3 from Jörg Höhle hoehle@users.sourceforge.net 2011-11-01 04:51:14 CDT --- The buffer memory leak is plugged by http://source.winehq.org/git/wine.git/commit/41c6ffea440f0814b16b3499f4d7dbd...
Wine still leaks memory when one of the OS calls like EnqueueBuffers fails, esp. in capture mode.
http://bugs.winehq.org/show_bug.cgi?id=28023
--- Comment #4 from Jörg Höhle hoehle@users.sourceforge.net 2012-01-15 16:26:39 CST --- The leak will be plugged by my next patch submission. (hmm, I just noticed that IAC_Initialize will not release all resources in all error paths.)
However, there are new issues. A deadlock in test_session. hr = IAudioClient_Stop(cap_ac); In my extended render tests, test_session follows test_clock(0)/*exclusive mode*/. The machine will deadlock both cores most of the time with wine-1.3.37, yet I've not managed to throw winegdb at it. When I instead moved test_clock(0) in front of test_audioclient(), the machine managed to run 141 iterations before receiving E_FAIL from IARC_ReleaseBuffer. I suspect OSSpinLock and queue management.
http://bugs.winehq.org/show_bug.cgi?id=28023
Jörg Höhle hoehle@users.sourceforge.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution| |FIXED
--- Comment #5 from Jörg Höhle hoehle@users.sourceforge.net 2012-01-16 14:30:46 CST --- Leak fixed by commit commit 89eaa56a2f3e42ed3979722540930ee0c6058abf and earlier ones. Should Enqueue fail, This->public_buffer is preserved and winecoreaudio relies on AUDCLNT_E_DEVICE_INVALIDATED to have the app Release and reopen the device in the hope that it'll succeed nex time.
What we have in none of the 3 drivers is a mechanism to permanently signal an error like DEVICE_INVALIDATED to the next GetBuffer or GetCurrentPadding.
The deadlock is still present, but that's another issue.
http://bugs.winehq.org/show_bug.cgi?id=28023
--- Comment #6 from Jörg Höhle hoehle@users.sourceforge.net 2012-01-18 16:18:48 CST --- When I'll be back at my Mac, I'll investigate whether the cause of deadlocks truly is OSSpinLock in ca_in/out_buffer_cb. Then I'll switch to InterlockedPushEntrySList (OSAtomicFifoDequeue is only in MacOS 10.7) and use EnterCriticalSection for the rest of the code.
Any other idea?
This would also get rid of the anti-pattern IMHO OSSpinLockUnlock(&This->lock); sc = AudioQueueFlush(This->aqueue); Readers of wine-devel will remember that I've criticized use of mid-function LeaveCS; Wait; EnterCS. Don't release a lock mid-way. http://www.winehq.org/pipermail/wine-devel/2011-June/090616.html
http://bugs.winehq.org/show_bug.cgi?id=28023
Jörg Höhle hoehle@users.sourceforge.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29657
http://bugs.winehq.org/show_bug.cgi?id=28023
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #7 from Alexandre Julliard julliard@winehq.org 2012-01-27 14:16:40 CST --- Closing bugs fixed in 1.4-rc1.
-
wine-bugs＠winehq.org