Opened 10 years ago

Closed 7 years ago

Last modified 7 years ago

#10744 closed defect (wontfix)

GetModuleHandle() Window API buggy on Windows Vista.

Reported by: Jurko Owned by:
Priority: normal Milestone:
Component: wxMSW Version:
Keywords: WindowsAPI Vista Cc:
Blocked By: Blocking:
Patch: yes

Description

With wxWidgets 2.8.9 & 2.8.10 (but the problem has most likely present in all versions) there is a problem with code that attempts to get a handle of an already loaded DLL (such as 'comctl32.dll') in order to acquire a pointer some API from it if available.

For example, such a problem got detected with wxWidgets 2.8.10 in 'src/mwc/window.cpp' in 'wxWindowMSW::HandleMouseMove()' where 'wxLoadedDLL dllComCtl32(_T("comctl32.dll"));' fails and reports an error.

In version 2.8.9 this used to be implemented using the wxDynamicLibrary class instead of wxLoadedDLL and then it would cause access violations if wxDynamicLibrary loaded a new DLL instead of just temporarily bumping up the reference count on an already loaded one. I guess the change included in the 2.8.10 release was intended to solve this problem but it just changed the way the problem gets manifested. Admittedly an error message is a better way of handling this than an access violation but still does not make everything 'just work'.

Our 'educated' guess is that on Windows Vista there is a bug with the ::GetModuleHandle() Windows API causing it to fail when not called without specifying the module's full path information. This has been detected for instance on a clean Windows Vista Ultimate installation with full Windows Updates applied. This would explain both why the problematic ::LoadLibrary() calls in wxWidgets 2.8.9 were loading new modules and why the 'fix' in wxWidgets 2.8.10 does not work either.

Attached patch works around this by attempting to acquire a module's handle 'manually' in case the module was identified without its full path information and the original ::GetModuleHandle() call fails. It then goes through the list of all modules loaded by the current process and does the module name comparison itself. This way has been tested to work on all Windows versions available to us.

We prepared separate patches for the the trunk, the 2.8 and 2.9.0 branches.


Patch description for a commit message:

Workaround for a problem where the ::GetModuleHandle() Windows API does not return a correct DLL module handle when the module has been specified using its name but without full path information. MSDN documentation states that this should work but unfortunately in such cases it does not. This has been detected on some installations of Windows Vista Ultimate when attempting to load the comctl32.dll.

The same background problem may also be at the cause of some strange ::LoadLibrary() Windows API behaviour detected on Windows Vista where it will sometimes load a new library even when a library with the same name is already loaded.

The patch itself has some room for improvements. For instance, some generic Windows API wrapper code in the src\msw\dlmsw.cpp unit seems like it should be moved to the include\wx\msw\private.h header file.


Attachments (10)

fix_for_fetching_handles_to_modules_that_are_already_loaded__2.8__60386.patch download (11.7 KB) - added by Jurko 10 years ago.
Patch for the 2.8 branch, based on revision 60386
fix_for_fetching_handles_to_modules_that_are_already_loaded__2.9.0__60386.patch download (11.6 KB) - added by Jurko 10 years ago.
Patch for the 2.9.0 branch, based on revision 60386
fix_for_fetching_handles_to_modules_that_are_already_loaded__trunk__60386.patch download (11.6 KB) - added by Jurko 10 years ago.
Patch for the trunk, based on revision 60386
GetModuleHandle_Windows_API_bug.7z download (8.6 KB) - added by Jurko 10 years ago.
Projects & registry entries needed for reproducing the problem.
patching_Vista_advapi32_for_running_previous_bug_example_projects.7z.001 download (195.3 KB) - added by Jurko 10 years ago.
Windows Vista advapi32.exe patch needed to run unsigned CSPs (needed for the previous example code) - part 1
patching_Vista_advapi32_for_running_previous_bug_example_projects.7z.002 download (110.5 KB) - added by Jurko 10 years ago.
Windows Vista advapi32.exe patch needed to run unsigned CSPs (needed for the previous example code) - part 2
fix_for_fetching_handles_to_modules_that_are_already_loaded__2.8.patch download (10.7 KB) - added by Jurko 10 years ago.
Corrected patch for the 2.8 branch.
fix_for_fetching_handles_to_modules_that_are_already_loaded__2.9.0.patch download (10.6 KB) - added by Jurko 10 years ago.
Corrected patch for the 2.9.0 branch.
fix_for_fetching_handles_to_modules_that_are_already_loaded__trunk.patch download (10.6 KB) - added by Jurko 10 years ago.
Corrected patch for the trunk.
Vista_GetModuleHandle_bug_demo.7z download (3.9 KB) - added by Jurko 10 years ago.
Simplest reproducing use case available so far.

Download all attachments as: .zip

Change History (20)

Changed 10 years ago by Jurko

Patch for the 2.8 branch, based on revision 60386

Changed 10 years ago by Jurko

Patch for the 2.9.0 branch, based on revision 60386

Changed 10 years ago by Jurko

Patch for the trunk, based on revision 60386

comment:1 Changed 10 years ago by vadz

  • Status changed from new to infoneeded_new

Thanks for your patches but before looking at in details I'd really like to understand what the original problem is. I.e. how can I reproduce the bug with current svn trunk?

TIA!

comment:2 Changed 10 years ago by Jurko

  • Status changed from infoneeded_new to new

Unfortunately this bug is not so easy to reproduce. :-(

I've attached a small example reproducing this behaviour in Windows, but it takes a bit of effort to set up and so is not really suitable for any kind of automated testing. The source code should be extremely easy to understand though.

First of all, this is a Windows bug and not really a wxWidgets bug, so whether you use the trunk or not is not really important. The wxWidgets code affected by this bug is easy to find (all calls to the ::GetModuleHandle() and ::LoadLibrary() Windows APIs) and manually review to see what the effects of the bug would be. If you need to actually see wxWidgets misbehave due to this bug, modify the testGetModuleHandle() function in dummyCSP to display a wxWidgets dialog containing a text widget and then try moving the mouse over it. That should cause an access violation in wxWidgets 2.8.9 or an error message about failing to locate the comctl32.dll DLL in later versions.

To set up & run the example you need to do the following:

  1. Do all of this on Windows Vista. We have not tested on editions other than Ultimate and have not tested new beta Windows 7.
  1. Compile the included DummyCSP C++ project (MSVS 9.0 C++ project files included) building a dummy CSP module. That is a DLL conforming to Microsoft's Crypto Service Provider interface. All of its functions have been coded so they fail (i.e. return FALSE) and its initial CPAcquireContext() function also calls the testGetModuleHandle() function which tries to get a handle to the loaded comctl32.dll DLL.
  1. When you build this CSP you will need to register it with Windows. You can do this by updating the included dummyCSP.reg registry import file so it contains the correct path to your built DLL and importing it to the registry. It consists of a single registry key with three simple values, one of which is the path.
  1. In order to be able to use an unsigned CSP on your system you will need to replace your system advapi32.dll with a version that has been 'cracked' so it does not verify CSP signatures. The crack is a simple 4 byte change that just disables this one check. And no, Microsoft provides no other way to develop CSPs other than first asking them to sign it, which of course is not a real quick way to develop software. :-) Attachment includes the patched advapi32.dll version for Windows Vista together with the original version - for your reference. To replace it give yourself full access privileges to the original advapi32.dll DLL, rename it to advapi32.dll.original, copy the patched version under the name advapi32.dll and then reboot your computer. Similar procedure reverts the change.
  1. Now you need to compile the attached C# DummyCSPLoader project. You can use .NET library version 2.0 or newer. This is a simple project that just loads the prepared dummy CSP and effectively calls its CPAcquireContext() function. Unfortunately this was the easiest way we found to reproduce this behaviour. I am not really sure what .NET does special here but we failed to reproduce the bug by coding this in C++ using Microsoft's CryptoAPI even though that is the exact same underlying API used by the .NET framework's RSACryptoServiceProvider class.
  1. Now run the built dummyCSPLoader.exe from outside the MSVS C# IDE. If you run it directly from the MSVS C# IDE - all works fine. :-) It could be that they have some sort of an 'uninitialized variable' problem covered up by IDE's memory initialization.
  1. dummyCSPLoader.exe will ask you to press a key before continuing so you can easily attach an external debugger to it. You can do so here if you want to set breakpoints in and debug the DummyCSP. Attaching an external debugger like this does not prevent the bug from appearing. You can use the debugger to verify that the program actually has the comctl32.dll loaded and to see ::GetModuleHandle("comctl32.dll") calls actually return zeros (invalid handle values) and lie about not being able to find the comctl32.dll DLL.
  1. Now dummyCSPLoader.exe will attempt to acquire a DummyCSP context which should fail but not before running the testGetModuleHandle() function displaying whether the comctl32.dll module handle can be acquired. If no messages pop up then the CSP did not get loaded, either because you failed to patch the advapi32.dll or because you forgot to register the CSP in the Windows registry or because you registered it with an incorrect path.

We'll let you know if we manage to find a simpler test case for reproducing this but this is the simplest we managed so far. More convoluted examples use Internet Explorer, SSL, require a much smarter CSP implementation and depend heavily on which web pages you visited before running the test so consider yourself lucky... :-)

Hope this helps.

Best regards,

Jurko Gospodnetić

Changed 10 years ago by Jurko

Projects & registry entries needed for reproducing the problem.

Changed 10 years ago by Jurko

Windows Vista advapi32.exe patch needed to run unsigned CSPs (needed for the previous example code) - part 1

Changed 10 years ago by Jurko

Windows Vista advapi32.exe patch needed to run unsigned CSPs (needed for the previous example code) - part 2

comment:3 Changed 10 years ago by Jurko

I've attached corrected patch versions for all three branches.

The original patches had two problems:

  1. Did not correctly convert module names to lowercase in Unicode builds.
  2. 'Off by one' error when detecting where the base file name begins in a given path.

Changed 10 years ago by Jurko

Corrected patch for the 2.8 branch.

Changed 10 years ago by Jurko

Corrected patch for the 2.9.0 branch.

Changed 10 years ago by Jurko

Corrected patch for the trunk.

comment:4 Changed 10 years ago by Jurko

We finally managed to get s simpler use-case reproducing this behaviour. It still uses a .NET component but at least no longer requires patching your system DLLs to run it or has any references to cryptography. :-)

I'm attaching a 7-zip archive containing two projects:

  1. DLL - A simple Windows DLL project (prepared using Microsoft Visual Studio 9.0 Express Edition for C++) containing a single exported function called testGetModuleHandle(). It simply tries to access the 'comctl32.dll' handle and displays whether it succeeded using a standard Windows message box.
  1. DLLLoader - A simple C# .NET console project (repared using Microsoft Visual Studio 9.0 Express Edition for C#. Loads the aforementioned DLL (expects to find it on the path or in the same folder where the C# project's .exe is located) and calls its testGetModuleHandle() function. It first waits for the user to press any key so you have a chance to hook an external debugger to this process so you can make sure that the comctl32.dll is actually loaded into the process even though ::GetModuleHandle("comctl32.dll") fails.

As before, the bug will only be demonstrated when running the DLLLoader project from outside its IDE. If you run it from inside the Visual Studio IDE, ::GetModuleHandle("comctl32.dll") will not fail.

When we find some more time we might try again to reproduce this without using .NET at all, but so far all such attempts failed. It could even be that this only fails in applications using managed code or something equally silly.

Hope this helps.

Best regards,

Jurko Gospodnetić

Changed 10 years ago by Jurko

Simplest reproducing use case available so far.

comment:5 Changed 10 years ago by Jurko

  • Version 2.8-svn deleted

I'm removing this patch's version information since it is actually related to multiple versions. I also hope this pings the maintainers so they look into the patch and possibly apply it or provide feedback. :-) *gdr*

comment:6 follow-up: Changed 8 years ago by vadz

I really don't know what to do about this one, I still find it hard to fathom that such bug could exist in Windows itself so I strongly suspect that it's some build/manifest problem which results in a wrong version of comctl32.dll being loaded or something else wx-specific. I'll leave the patch here in case anybody else runs into this problem but I don't think we're going to apply it to the official version without either understanding more about this bug or getting more bug reports about its occurrences.

comment:7 in reply to: ↑ 6 Changed 8 years ago by Jurko

  • Resolution set to fixed
  • Status changed from new to closed

Hi vadz.

Have you tried reproducing the problem using the example I posted?

As I recall, we tried real hard to make the example as simple as possible. It does not reference wxWidgets at all, it should have no explicit comctl32.dll references using full paths and the build settings used should be as close to default as possible. We even compiled them using free tools. :-)

We've been using this patch in out local wxWidgets source tree and there have been no further problems since it has been applied.

When I get some free time next, I'll try to look into the problem again and see if the latest Windows Vista updates perhaps patched something on the Windows side. And I'll try to test the same problem on Windows 7.

Hope this helps.

Best regards,
Jurko Gospodnetić

comment:8 Changed 8 years ago by Jurko

  • Resolution fixed deleted
  • Status changed from closed to reopened

Err... reverting status as I managed to change it before by mistake. Sorry for the noise.

comment:9 Changed 7 years ago by vadz

  • Resolution set to wontfix
  • Status changed from reopened to closed

I finally tested the bug demo and it doesn't work for me. Here is what I did ("%" is my shell prompt):

% cl /LDd testGetModuleHandle.cpp /link user32.lib
testGetModuleHandle.cpp
   Creating library testGetModuleHandle.lib and object testGetModuleHandle.exp
% csc /platform:x86 Program.cs
% ./Program.exe
Press any key to start (opportunity to attach a debugger).

After this the first message box (for GetModuleHandleA()) fails, as expected because there is no reason for comctl32.dll to be loaded at this point. Somewhat more surprisingly, the second one (for GetModuleHandleW()) succeeds but I guess this is because showing the message box actually loads comctl32.dll -- and, in fact, if I exchange the two calls, then the first one always fails while the second one succeeds.

If I add a line

LoadLibrary("comctl32");

to the beginning of testGetModuleHandle() function, both calls succeed.

The behaviour is the same under Vista and 7, I didn't test under XP.

All I can say is to repeat that the problem seems to be specific to your machine or your environment, this bug is really too gross to be unnoticed and unheard of if it really existed.

Sorry for the time you spent on this but I just can't convince myself that this is worth adding all this extra code to wx.

comment:10 Changed 7 years ago by Jurko

Ah well. No biggie... applied the patch in the project where this was originally encountered and the project was completed successfully (now a 'long' time ago). :-)

As I recall we reproduced the problem on multiple Windows Vista machines. Let's hope M$ fixed this with some later update. If someone else encounters the problem - at least they'll have the data in this ticket to start from.

Thanks for taking the time to process the report and reply!

Best regards,

Jurko Gospodnetić

Note: See TracTickets for help on using tickets.