Deadlock - library scan vs track playback (Win)

[kramer]

(Note: I was testing the master branch when I meant to be testing NewSoundSourceAPI. I will try to reproduce on the desired branch.)

Similar to #1445298, this affects M4A/AAC. I was playing an AAC file, then clicked "Rescan Library", then tried to load another AAC file. This apparently caused a deadlock. I verified that both threads were blocked on the same file in the debugger. The trace of the library scanner thread:

  ntdll.dll!77c9015d()
  [Frames below may be incorrect and/or missing, no symbols loaded for ntdll.dll]
  ntdll.dll!77c9015d()
  KernelBase.dll!75a815e9()
  KernelBase.dll!75a8151f()
  kernel32.dll!758119f8()
  mfreadwrite.dll!69020162()
  mfreadwrite.dll!6901bef1()
  mfreadwrite.dll!6901a53b()
  mfreadwrite.dll!6901a8d5()
> soundsourcemediafoundation.dll!SoundSourceMediaFoundation::read(unsigned long size, const short * destination) Line 225 + 0x26 bytes C++
  mixxx.exe!AnalyserQueue::doAnalysis(QSharedPointer<TrackInfoObject> tio, const QSharedPointer<Mixxx::SoundSource> & pSoundSource) Line 177 + 0x24 bytes C++
  mixxx.exe!AnalyserQueue::run() Line 332 + 0x1a bytes C++
  QtCored4.dll!QThreadPrivate::start(void * arg) Line 357 C++
  msvcr120d.dll!63e73651()
  msvcr120d.dll!63e73861()
  kernel32.dll!7581338a()
  ntdll.dll!77ca9f72()
  ntdll.dll!77ca9f45()

The trace of the other thread (same as #1445298):

  ntdll.dll!77c9015d()
  [Frames below may be incorrect and/or missing, no symbols loaded for ntdll.dll]
  ntdll.dll!77c9015d()
  KernelBase.dll!75a815e9()
  KernelBase.dll!75a8151f()
  kernel32.dll!758119f8()
  mfreadwrite.dll!6bf59f93()
  mfreadwrite.dll!6bf5920e()
> soundsourcemediafoundation.dll!SoundSourceMediaFoundation::seek(long filepos) Line 167 + 0x1d bytes C++
  mixxx.exe!CachingReaderWorker::processChunkReadRequest(ChunkReadRequest * request, ReaderStatusUpdate * update) Line 71 C++
  mixxx.exe!CachingReaderWorker::run() Line 115 C++
  QtCored4.dll!QThreadPrivate::start(void * arg) Line 357 C++
  msvcr120d.dll!69053651()
  msvcr120d.dll!69053861()
  kernel32.dll!7581338a()
  ntdll.dll!77ca9f72()
  ntdll.dll!77ca9f45()

[Daniel Schürmann (daschuer)]

Buh! It looks like SoundSourceMediaFoundation/mfreadwrite.dll is not thread save.

Maybe it is caused by
https://github.com/mixxxdj/mixxx/blob/bba85f15540e277af6fbf343fda93495c5090c6f/plugins/soundsourcemediafoundation/soundsourcemediafoundation.cpp#L102

Here is is recommendation to use COINIT_MULTITHREADED
https://msdn.microsoft.com/de-de/library/windows/desktop/ee892371%28v=vs.85%29.aspx

However in other place you found advises that you have to use COINIT_APARTMENTTHREADED
https://msdn.microsoft.com/de-de/library/windows/desktop/dd757929%28v=vs.85%29.aspx

Even if setting COINIT_MULTITHREADED fixes the issue,
It will likely enable an extra locking layer. That may cause a priority inversion between th low priority analyzer thread and the High priority caching worker thread.

The issue seams to be difficult. Maybe it helps switching to the Callback interface.
https://msdn.microsoft.com/de-de/library/windows/desktop/ms704787%28v=vs.85%29.aspx
But that is not a short term solution.

[Daniel Schürmann (daschuer)]

Thank you for looking at this!
I'm am curious to to hear how COINIT_MULTITHREADED makes a difference.

Is Bug #1445298 a duplicate?

[kramer (default-kramer)]

They are certainly related, but may not be an exact duplicate. The difference is in Bug 1445298, none of the other threads in Mixxx were touching the AAC file - they all looked "normal" (e.g. in a QT or OS wait). In this bug, it was clear that two Mixxx threads were deadlocked on the same AAC file.

[Uwe Klotz (uklotzde-deactivatedaccount)]

Very likely to be fixed with the new SoundSource framework that will be introduced with Mixxx 2.1. Includes a rewrite of SoundSourceMediaFoundation.

[Daniel Schürmann (daschuer)]

Good news.
Could you explain why it is fixed?
Do we use mfreadwrite.dll from a single thread at a time now or did we face different issue.
Thanks.

[Uwe Klotz (uklotzde-deactivatedaccount)]

I'm not able to reproduce this issue, even when playing the same track on multiple decks and reanalysing it concurrently.

I don't think that we still have any issues here. The initialization and usage of the COM runtime and interfaces has been fixed including potential reference leaks. COINIT_APARTMENTTHREADED seems to be appropriate, because we don't provide any MF components ourselves that need to run in an MTA. The COM runtime is initialized within every thread and those different threads should not get into the way of each other.

Please don't mind if I do not spend any more time to analyse the old, buggy code base. Let's see how 2.1 will perform and create a new bug report if we need to.

[Daniel Schürmann (daschuer)]

Thank you for the update. This is a good plan.

[Swiftb0y (swiftb0y)]

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/7968