New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 166397 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2013
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

CoreAudioUtil::CreateDeviceEnumerator()

Reported by xians@chromium.org, Dec 17 2012

Issue description

Version: 
OS: windows 7


Hi There,

I searched the go/crash, and found some crash logs about the device listener in media, for example:
https://crash.corp.google.com/reportdetail?reportid=73a8ac4b6f5b4c6b

It seems the the crash happens in CreateDeviceEnumerator:

0x65316f48	 [chrome.dll]	 - debugger_win.cc:107]	base::debug::BreakDebugger()
0x64e5d952	 [chrome.dll]	 - core_audio_util_win.cc:159]	media::CoreAudioUtil::CreateDeviceEnumerator()
0x64e5d73e	 [chrome.dll]	 - audio_device_listener_win.cc:25]	media::AudioDeviceListenerWin::AudioDeviceListenerWin(base::Callback<void (void)> const &)
0x64e5d2de	 [chrome.dll]	 - audio_manager_win.cc:146]	media::AudioManagerWin::CreateDeviceListener()
0x64db14c7	 [chrome.dll]	 - message_loop.cc:473]	MessageLoop::RunTask(base::PendingTask const &)

And similar crashes can be found in different 25.x canaries, so I guess it has been there for quite a while.
Dale and Henrik, could you guys take a look at the logs and see if we need to fix it for M25? 


 
Shijing,

I have done lots of work earlier trying to sort this out but it seems like COM init fails on some devices and then we crash in the device listener (since it is a very early user of COM). Even if we removed the CHECK; most likely any other COM-related task would fail a t a later stage. I have discussed this with tommi@ as well but I have no solid solution.
Cc: tommi@chromium.org xians@chromium.org
Input from Shijing:

From what I can see from the log, we failed on creating the windows enumerator, this might be due to some COM initialization issue. If this still happens with the M25 beta, I would propose the following alternative solutions:

#1 Do not start the listener when chrome launches, instead, start it from the first time a audio stream is created or the first device enumeration. By doing so, we can reduce the startup time and also avoid some OS setup issues.

#2 Since AudioInputDeviceManager also uses the similar code to create its own windows enumerator, and we do not see problem there, and it is done after IO thread is created. So I believe if we create the AudioManager after IO thread is created, it should fix the crash. This can be easily done by moving the AudioManager creation code to BrowserMainLoop::BrowserThreadsStarted(), right before media_stream_manager_ is created.

Preferably, #2 is better than #1, but #1 is a quick fix with least change.
Cc: scherkus@chromium.org
Comments from henrika:

#1 To me that sounds like we would only delay the issue until the first audio stream is created. Might be seen as an improvement but you might end up with a more error prone design where a "singleton pattern" is needed.

#2 Isn't the reason why you don't see the problem simply that we never get that far on the problematic machines since we hit a CHECK earlier? I don't understand how an existing IO thread could possible affect the COM state of the audio manager thread.

Comment 4 by tommi@chromium.org, Dec 20 2012

To refresh my memory, is this the CHECK that's failing?

  ScopedComPtr<IMMDeviceEnumerator> device_enumerator;
  HRESULT hr = CoCreateInstance(__uuidof(MMDeviceEnumerator),
                                NULL,
                                CLSCTX_INPROC_SERVER,
                                __uuidof(IMMDeviceEnumerator),
                                device_enumerator.ReceiveVoid());
  // CO_E_NOTINITIALIZED is the most likely reason for failure and if that
  // happens we might as well die here.
  CHECK(SUCCEEDED(hr));

If so, I think we need to make sure it's actually CO_E_NOTINITIALIZED that we're running into and not something else.
It's possible that this information is already in the crash dumps, so let's try to figure that out.

If it turns out that we are not running into CO_E_NOTINITIALIZED, then COM initialization is succeeding (and I suspect this is the case), and we need to look for other possibilities.  I ran into problems with the Media Foundation API which could be similar to this one.  The problem was that the API wasn't available on all versions of Windows on which it is supported.  For "Windows N" for instance, the API is not available.  Could that be what we're seeing?
Today:

ScopedComPtr<IMMDeviceEnumerator> CoreAudioUtil::CreateDeviceEnumerator() {
  DCHECK(CoreAudioUtil::IsSupported());
  ScopedComPtr<IMMDeviceEnumerator> device_enumerator;
  HRESULT hr = CoCreateInstance(__uuidof(MMDeviceEnumerator),
                                NULL,
                                CLSCTX_INPROC_SERVER,
                                __uuidof(IMMDeviceEnumerator),
                                device_enumerator.ReceiveVoid());
  // CO_E_NOTINITIALIZED is the most likely reason for failure and if that
  // happens we might as well die here.
  CHECK(SUCCEEDED(hr));
  return device_enumerator;
}

What about?

ScopedComPtr<IMMDeviceEnumerator> CoreAudioUtil::CreateDeviceEnumerator() {
  DCHECK(CoreAudioUtil::IsSupported());
  ScopedComPtr<IMMDeviceEnumerator> device_enumerator;
  HRESULT hr = CoCreateInstance(__uuidof(MMDeviceEnumerator),
                                NULL,
                                CLSCTX_INPROC_SERVER,
                                __uuidof(IMMDeviceEnumerator),
                                device_enumerator.ReceiveVoid());
  if (FAILED(hr)) {
    // CO_E_NOTINITIALIZED is the most likely reason for failure and if that
    // happens we might as well die here.
    // We're about to crash (CHECK).
    // Use Alias in hopes that it makes it into crash dumps.
    base::debug::Alias(&hr);
    CHECK(false);
  }
  return device_enumerator;
}

Comment 6 by tommi@chromium.org, Dec 20 2012

It's enough to just update the comment (since it incorrectly implies CO_E_NOTINITIALIZED) and add one line:

  base::debug::Alias(&hr); <- just add this.
  CHECK(SUCCEEDED(hr));
  return device_enumerator;

However, I looked at the dump and found that the error is 80040154 or REGDB_E_CLASSNOTREG.  That means that this class isn't available on the particular machine.

I.e. we need to do a more rigorous test to see if this API is really supported, possibly similar to what was needed for the Media Foundation API.

Thanks Tommi. Any references to suitable actions? I have found usage of REGDB_E_CLASSNOTREG at some places in Chrome but it seems to be ignored in all cases I can find:

e.g.

if (hr != REGDB_E_CLASSNOTREG) {
      LOG(ERROR) << "Unexpected result creating CommandExecuteImpl; hr=0x"
                 << std::hex << hr;
}

Comment 10 by tommi@chromium.org, Dec 20 2012

REGDB_E_CLASSNOTREG means the the class is not available.  So, we should treat it as the API not being supported on the machine and fall back on an alternate API or disable audio entirely.
Perhaps we could continue offline but it is not clear to me if we should turn of device enumeration only or if we can safely assume that any future Core Audio related factory API will fail with the same result.
Labels: Feature-Media
Tommi and I have discussed the issue and have a potential solution based on extending the existing CoreAudioUtil::IsSupported() method by ensuring that a key DLL for Core Audio can be loaded (one that is the "mother of all Core Audio DLLs"). It seems like some Vista and Win 7 machines are "damaged" and does not contain the required DLL. On these machines we must revert back to WaveXX instead and this decision shall be taken as early as possible so we don't start COM-initializing threads on devices which does not support the COM classes we need.
Owner: henrika@chromium.org
Status: Started

Comment 16 by tommi@chromium.org, Dec 21 2012

Just so that there's no misunderstanding - these installations are not "damaged".  We don't support damaged installations. If it were the case that the installation was damaged, we wouldn't be fixing this (since who knows what else might be wrong).

What's more likely is that the API(s) isn't available such as can be the case for the Media Foundation API.
Got it. Bad word perhaps.

My point was that a missing API may very well come from the fact that a user has been fiddling with the essential DLLs (http://www.topdll.com/download/AudioSes.dll?q=afbb5060a2dad431a2eaeb2c86cffe81).
Project Member

Comment 18 by bugdroid1@chromium.org, Dec 21 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=174373

------------------------------------------------------------------------
r174373 | henrika@chromium.org | 2012-12-21T12:04:09.457839Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/audio/win/core_audio_util_win.cc?r1=174373&r2=174372&pathrev=174373

Improves CoreAudioUtil::IsSupported by also loading Audioses.dll DLL


BUG= 166397 
TEST=media_unittests and manual tests in Chrome where I force DLL load to fail.

Review URL: https://chromiumcodereview.appspot.com/11666005
------------------------------------------------------------------------
Status: Fixed
Status: Assigned
henrika are you sure this is fixed? I'm still seeing crashes in recent builds:

Crashes in 26.0.1388.0
1ea41207058d3ffa
2e112aaa18e99664
3016f8fe572161b2
5eb0b85915d48443
5f000f327a8fe3a2
b5e6c73d8ae7cc28
cd3c835880cd70e3
ce7ecc225a36c564
f972cefed652970a

168 ScopedComPtr<IMMDeviceEnumerator> CoreAudioUtil::CreateDeviceEnumerator() {
169   DCHECK(CoreAudioUtil::IsSupported());
170   ScopedComPtr<IMMDeviceEnumerator> device_enumerator;
171   HRESULT hr = CoCreateInstance(__uuidof(MMDeviceEnumerator),
172                                 NULL,
173                                 CLSCTX_INPROC_SERVER,
174                                 __uuidof(IMMDeviceEnumerator),
175                                 device_enumerator.ReceiveVoid());
176   // CO_E_NOTINITIALIZED is the most likely reason for failure and if that
177   // happens we might as well die here.
178   CHECK(SUCCEEDED(hr));    *** BOOM ***
179   return device_enumerator;
180 }
scherkus: Yes we still see the problem but based on the correlation of reports and additional analysis in WinDbg (tommi will add more details) I would say that we see the crash since we are "early in startup sequence in Chrome". That said, if we removed the CHECK above, the users would most likely crash at a later stage for some other reason, e.g. OOM.

I have tried to compare the ratio of browser crashes caused by CoreAudioUtil::CreateDeviceEnumerator() before and after the latest patch but only found that the ratio was actually lower before the patch: 9 out of 45596 after (26.0.138.0) and 1 out of 41938 before (used 26.0.1366).

In any case, I don't see that we can do anything else to prevent the issue.

Tommi@: anything to add?

Comment 22 by tommi@chromium.org, Jan 25 2013

Henrik is going to take a look at the number of crashes before and after the fix.  That should give us an idea about whether or not the fix actually works, and it should at least fix one class of errors.

Report 1ea41207058d3ffa is really strange since it crashes on 800401f0 or "CoInitialize has not been called" which we've gone out of our way to ensure never happens.  I'm taking a closer look at that one right now.

What remains, I'm not sure about.  I took a look at 2e112aaa18e99664 and there the problem is E_OUTOFMEMORY.  I don't think we can really do anything for that case.

Other things that seem to be common in the crash reports we're seeing are:
* The crash dumps often come from Russian users.  Possibly this is a group of programmers just hacking on Chrome.
* The process tends to have 3rd party "toolbars" installed.

Comment 23 by kareng@google.com, Feb 26 2013

Labels: -Pri-2 Pri-1 Mstone-27
Henrik I see this crashing on todays canary/dev

Thread 8 *CRASHED* ( EXCEPTION_BREAKPOINT @ 0x633c22a2 )

0x633c22a2	 [chrome.dll]	 - debugger_win.cc:107]	base::debug::BreakDebugger()
0x62ec974b	 [chrome.dll]	 - core_audio_util_win.cc:146]	media::CoreAudioUtil::CreateDeviceEnumerator()
0x62ec9686	 [chrome.dll]	 - audio_device_listener_win.cc:26]	media::AudioDeviceListenerWin::AudioDeviceListenerWin(base::Callback<void (void)> const &)
0x62ec94b0	 [chrome.dll]	 - audio_manager_win.cc:146]	media::AudioManagerWin::CreateDeviceListener()
0x62e1510e	 [chrome.dll]	 - message_loop.cc:476]	MessageLoop::RunTask(base::PendingTask const &)
0x62e14cc8	 [chrome.dll]	 - message_loop.cc:671]	MessageLoop::DoWork()
0x62e152c2	 [chrome.dll]	 - message_pump_default.cc:29]	base::MessagePumpDefault::Run(base::MessagePump::Delegate *)
0x77181b66	 [ole32.dll]	 + 0x00041b66]	CComApartment::CComApartment(tagAPTKIND)
0x00010000			
0x62e17adc	 [chrome.dll]	 - thread.cc:197]	base::Thread::ThreadMain()
0x62e179fe	 [chrome.dll]	 - platform_thread_win.cc:57]	base::`anonymous namespace'::ThreadFunc(void *)
0x766833a9	 [kernel32.dll]	 + 0x000133a9]	BaseThreadInitThunk
0x77c29ef1	 [ntdll.dll]	 + 0x00039ef1]	__RtlUserThreadStart
0x77c29ec4	 [ntdll.dll]	 + 0x00039ec4]	_RtlUserThreadStart

https://crash.corp.google.com/reportdetail?reportid=56353d7a47e62296#crashing_thread
Let's recap.

My latest patch in this area was https://chromiumcodereview.appspot.com/11666005 and it landed in 174373.

25.0.1364.8 <=> 174422
                       174373
25.0.1364.7 <=> 174330
Taking 27.0.1423.0 as an example, 10 out of 3395 browser crashes comes from this signature. Not all are unique (~5 different clients).

I am unable to figure out why we still see these crashes and the only possible "solution" I see is to remove the CHECK() and see where that leads. I would assume that it would only lead to a crash at a later stage and/or non-working audio for these clients.

I will discuss with tommi and see if there is anything we can do here.

Comment 26 by tommi@chromium.org, Feb 28 2013

Can you check the error we're seeing in these crash dumps?
(see previous updates for what the error we've seen.  Out of memory seems
to be common).
Looked at all ten logs and searched for unique users.

User #1 => 5 crashes
User #2 => 2 crashes
User #3 => 1 crash
User #4 => 2 crashes

*User #1:*
*
*

Error code: (HRESULT) 0x8007007e (2147942526) - The specified module could
not be found.

67120000 67156000   AudioSes   (deferred)
67160000 67199000   MMDevAPI   (deferred)

0:008> lmv m audioses
start    end        module name
67120000 67156000   AudioSes   (deferred)
    Mapped memory image file:
c:\code\symbols\AudioSes.dll\4CE7B72536000\AudioSes.dll
    Image path: C:\Windows\System32\AudioSes.dll
    Image name: AudioSes.dll
    Timestamp:        Sat Nov 20 12:55:17 2010 (4CE7B725)
    CheckSum:         0003CD0F
    ImageSize:        00036000
    File version:     6.1.7601.17514
    Product version:  6.1.7601.17514
    File flags:       0 (Mask 3F)
    File OS:          40004 NT Win32
    File type:        2.0 Dll
    File date:        00000000.00000000
    Translations:     0409.04b0
    CompanyName:      Microsoft Corporation
    ProductName:      Microsoft
It seems like we only see this issue for very few users and I am not that worried but will keep track of it and report back when new crashes are added.
Can you modify CoreAudioUtil::IsSupported() to attempt to create a DeviceEnumerator and if it fails to return false? This can be done once and cached to reduce overhead.

Alternatively if even WaveOut won't work in this situation we could set a flag which switches to the fake audio path.  It's possible just changing IsSupported() would cause this to happen automatically.

Either should be a better user experience than a CHECK failure which blows up the browser :)
Thanks for thinking outside the box Dale; sounds like a good idea worth
trying. Let me fix a CL on Monday when I am back.
Project Member

Comment 31 by bugdroid1@chromium.org, Mar 5 2013

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=186166

------------------------------------------------------------------------
r186166 | henrika@chromium.org | 2013-03-05T13:08:35.778858Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/audio/win/core_audio_util_win.cc?r1=186166&r2=186165&pathrev=186166

Improved CoreAudioUtil::IsSupported().

This patch adds one more test in CoreAudioUtil::IsSupported() and that is to be able to create an IMMDeviceEnumerator. We have seen some crashes where it does not seem to be sufficient to check that Audioses.dll can be loaded. I hope we are fine now.

BUG= 166397 
TEST=media_unittests and different audio clients in Chrome.

Review URL: https://chromiumcodereview.appspot.com/12378066
------------------------------------------------------------------------
Project Member

Comment 32 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Area-Internals -Feature-Media -Mstone-27 Cr-Internals-Media Cr-Internals M-27
Looking good! Lets merge this to M26 since it's the top media crasher.
Labels: Merge-Requested
Requesting to merge this fix to M26 since it is an isolated patch and it is a top media crasher.
Labels: -M-27 M-26
Labels: -Merge-Requested Merge-Approved
Project Member

Comment 37 by bugdroid1@chromium.org, Mar 13 2013

Labels: -Merge-Approved merge-merged-1410
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=187811

------------------------------------------------------------------------
r187811 | henrika@chromium.org | 2013-03-13T08:13:10.704903Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1410/src/media/audio/win/core_audio_util_win.cc?r1=187811&r2=187810&pathrev=187811

Merge 186166 "Improved CoreAudioUtil::IsSupported()."

> Improved CoreAudioUtil::IsSupported().
> 
> This patch adds one more test in CoreAudioUtil::IsSupported() and that is to be able to create an IMMDeviceEnumerator. We have seen some crashes where it does not seem to be sufficient to check that Audioses.dll can be loaded. I hope we are fine now.
> 
> BUG= 166397 
> TEST=media_unittests and different audio clients in Chrome.
> 
> Review URL: https://chromiumcodereview.appspot.com/12378066

TBR=henrika@chromium.org
Review URL: https://codereview.chromium.org/12823002
------------------------------------------------------------------------
Status: Fixed
Labels: Needs-Feedback
henrika@, Could you please provide us the repro steps? so that it will be easy for test team to verify this issue, if we can.

Thanks in advance.
Sorry, I should have added more comments before marking it as fixed. You will most likely never be able to repro this issue since it is a very rare crash which only happens for some users who most likely have modified their machines in such a way that native Windows APIs fails.

I would say that only way to confirm is to verify that we no longer see this crash.

Sign in to add a comment