----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/272/#review639 -----------------------------------------------------------
Ship it! - Oz On April 18, 2011, 6:04 a.m., Siana Gearz wrote: > > ----------------------------------------------------------- > 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. > > > 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 > >
_______________________________________________ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
