[Windows] Improper usage of RegisterClass(), CreateWindow() and GetModuleHandle() in our code base.

276 views
Skip to first unread message

Alex Pakhunov

unread,
Apr 19, 2012, 5:20:42 PM4/19/12
to chromium-dev
Hi,

I've noticed that RegisterClass(), CreateWindow(), GetModuleHandle() as well as their Ex variants are not used properly in many places in Chromium code base. This leads to problems like http://crbug.com/124091. I'm planning to create a CL that fixes most obvious issues but I thought rising other people's awareness of the issue will also be helpful.
  1. RegisterClass(),  RegisterClassEx(), CreateWindow() and CreateWindowEx() accept HMODULE that should be a handle of the module containing the window procedure. Here is explained why: http://blogs.msdn.com/b/oldnewthing/archive/2005/04/18/409205.aspx. The only exception are window classes registerd with CS_GLOBALCLASS flag. There are a few places in the code that pass GetModuleHandle(NULL) instead (i.e. the handle of the main executable) which leads to problems if, say, there are two DLLs that happen to register two window classes with the same name.

    The proper way of doing this is getting the module handle from the window procedure pointer via GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS). That leads to the second problem:

  2. By default GetModuleHandle() does not increment the reference counter of a module. GetModuleHandleEx() on the other hand, does increment the counter unless the GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT flag is passed. As you have guessed, this flag is not passed to many  GetModuleHandleEx() calls (nor FreeLibrary() is called) while it is clear from the code that that was the intent. 
Thanks,
Alex.

Jói Sigurðsson

unread,
Apr 20, 2012, 5:38:09 AM4/20/12
to alex...@google.com, chromium-dev
It's probably best if everybody uses GetModuleFromAddress() from
base/process_util.h. It has the bug you mentioned with not using the
GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT flag, but then at least
we can fix it in one place (I'm preparing a patch now).

Cheers,
Jói

> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-dev

Jói Sigurðsson

unread,
Apr 20, 2012, 5:40:38 AM4/20/12
to alex...@google.com, chromium-dev
> [snip]  It has the bug you mentioned with not using the

> GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT flag, but then at least
> we can fix it in one place (I'm preparing a patch now).

Looks like you beat me to it Alex! (code search wasn't showing the
update yet) :)

Cheers,
Jói


2012/4/20 Jói Sigurðsson <j...@chromium.org>:

Reply all
Reply to author
Forward
0 new messages