[opensource-dev] Review Request: STORM-1088: bundled winmm.dll shim crashes on start-up on some systems.

[Oz Linden]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/272/
-----------------------------------------------------------

(Updated April 18, 2011, 6:04 a.m.)


Review request for Viewer.


Changes
-------

linking jira issue


Summary
-------

STORM-1088: bundled winmm.dll shim crashes on start-up on some systems.

This patch tests the theory that according to MSDN, calls to LoadLibrary, GetProcAddress in DllMain are likely to create a race condition.


This addresses bug storm-1088.
    http://jira.secondlife.com/browse/storm-1088


Diffs
-----

  doc/contributions.txt f632f87bb71b 
  indra/media_plugins/winmmshim/forwarding_api.h f632f87bb71b 
  indra/media_plugins/winmmshim/forwarding_api.cpp f632f87bb71b 
  indra/media_plugins/winmmshim/winmm_shim.cpp f632f87bb71b 

Diff: http://codereview.secondlife.com/r/272/diff


Testing
-------

winmm.dll from Second Life 2.6.3 (226640) Apr 14 2011 12:32:01 (Second Life Beta Viewer) causes this viewer, and a number of prior versions, to crash at startup for me.
winmm.dll built by me from viewer-development source code also does, tested against the same viewer binary.

The version of winmm.dll with my patch, built by me and substituted into the installed viewer, causes the above mentioned viewer to start up without issue and function apparently properly - sound works (FMOD), media_plugin_webkit works, voice is audible, windows_volume_catcher of llqtwebkit is functional.
Same applies to Second Life 2.6.3 (226792) Apr 17 2011 07:01:06 (Second Life Developer), built by Oz Linden with a prior version of my patch.

media_plugin_quicktime functionality untested, voice capture untested.

The patch contains debug output, which causes the following messages in debugger:

INFO: LLAudioEngine::init: LLAudioEngine::init() AudioEngine successfully initialized
'SecondLifeBetaViewer.exe': Loaded 'C:\WINXP\system32\winmm.dll', Cannot find or open the PDB file
'SecondLifeBetaViewer.exe': Loaded 'C:\WINXP\system32\myokent.dll', Cannot find or open the PDB file
WINMM_SHIM.DLL: real winmm.dll initialized successfully
WINMM_SHIM.DLL: real winmm.dll initialized successfully
'SecondLifeBetaViewer.exe': Loaded 'C:\WINXP\system32\wdmaud.drv', Cannot find or open the PDB file
'SecondLifeBetaViewer.exe': Unloaded 'C:\WINXP\system32\wdmaud.drv'
'SecondLifeBetaViewer.exe': Loaded 'C:\WINXP\system32\wdmaud.drv', Cannot find or open the PDB file
'SecondLifeBetaViewer.exe': Unloaded 'C:\WINXP\system32\wdmaud.drv'
'SecondLifeBetaViewer.exe': Loaded 'C:\WINXP\system32\wdmaud.drv', Cannot find or open the PDB file
'SecondLifeBetaViewer.exe': Unloaded 'C:\WINXP\system32\wdmaud.drv'
'SecondLifeBetaViewer.exe': Loaded 'C:\WINXP\system32\wdmaud.drv', Cannot find or open the PDB file
'SecondLifeBetaViewer.exe': Unloaded 'C:\WINXP\system32\wdmaud.drv'
'SecondLifeBetaViewer.exe': Loaded 'C:\WINXP\system32\wdmaud.drv', Cannot find or open the PDB file
'SecondLifeBetaViewer.exe': Unloaded 'C:\WINXP\system32\wdmaud.drv'
'SecondLifeBetaViewer.exe': Loaded 'C:\WINXP\system32\wdmaud.drv', Cannot find or open the PDB file
'SecondLifeBetaViewer.exe': Loaded 'C:\WINXP\system32\msacm32.drv', Cannot find or open the PDB file
'SecondLifeBetaViewer.exe': Loaded 'C:\WINXP\system32\midimap.dll', Cannot find or open the PDB file
'SecondLifeBetaViewer.exe': Loaded 'C:\WINXP\system32\ksuser.dll', Cannot find or open the PDB file
INFO: idle_startup: Audio Engine Initialized.

This is very puzzling to me, why the success message is being output twice. Perhaps if the reason of that is found, the real reason for the failure of failure of upstream winmm shim would become evident, or perhaps it's a bug in my patch. I'd like everyone to check whether they see the same thing.

...

After some testing, it turns out myokent.dll (MIDI Yoke, a software loop-back cable for musicians) seems to be the culprit on my system, causing a trivial circular dependency, not a race condition. It tries to use a number of winmm.dll functions (first of them being GetDriverModuleHandle) right from its DllMain, which is invoked by LoadLibrary. Now it turns out, that if we try to load real winmm.dll from DllMain, which in turn loads myokent.dll from its DllMain, which in turn from its DllMain uses winmm.dll's functions, calling the ones in OUR winmm.dll, which simply aren't initialized at that point yet because we need LoadLibrary (and DllMains of used DLLs) to complete, then we can populate the function pointers. My patch apparently sidesteps this trivial problem, but this doesn't give an insight on why, on systems with UAC, for some people, winmm.dll shim would cause a problem when run as a normal user, and not cause a problem when run with admin rights.

Still, if we assume the other problems on affected systems are similar in nature (circular dependency), then it appears we still need to move loading and initialization outside of DllMain to resolve them. Scratch that, if the problem is only this, it would be sufficient to first call LoadLibraryEx with DONT_RESOLVE_DLL_REFERENCES, then call init_function_pointers, then have a normal LoadLibrary call to kick off the rest of the chain.


Thanks,

Siana

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110418/7dcf3177/attachment-0001.htm