Skip to content

Borrowed HMODULE from GetModuleHandle is stored in an owning wil::unique_hmodule #142

Description

@JustinMDotNet

Summary

PluginSystem::Load always probes the host executable for RED4ext exports. For .exe paths it obtains the module with GetModuleHandleA(nullptr) and immediately stores the result in wil::unique_hmodule, which calls FreeLibrary on destruction. GetModuleHandleA returns a borrowed handle that does not bump the loader reference count, and Windows explicitly says it must not be passed to FreeLibrary.

Where

  • src/dll/Systems/PluginSystem.cpp:171-172 — every startup runs Load(m_paths.GetExe(), false) to probe the host exe.
  • src/dll/Systems/PluginSystem.cpp:236-240:
    wil::unique_hmodule handle;
    if (aPath.extension() == L".exe")
        handle.reset(GetModuleHandleA(nullptr));
    else
        handle.reset(LoadLibraryEx(aPath.c_str(), nullptr, flags));
  • src/dll/PluginBase.hpp:37-38PluginBase stores the handle in another wil::unique_hmodule m_module;.

Why it matters

Two distinct unbalanced FreeLibrary calls every startup:

  1. If the host .exe does not export Supports, the local handle goes out of scope and wil::unique_hmodule calls FreeLibrary(GetModuleHandle(nullptr)). The loader refcount on the process image is wrong by one in the negative direction.
  2. If the host .exe is accepted as a plugin (the hosted-executable path), PluginBase::m_module will eventually call FreeLibrary on the same borrowed handle on plugin unload.

Per Microsoft's documentation for GetModuleHandle:

The GetModuleHandle function does not increment a module's reference count, so this handle should not be passed to the FreeLibrary function...

Observable consequences range from "loader bookkeeping is silently off" to "process image gets unmapped at the wrong time" depending on what else is doing LoadLibrary / FreeLibrary on the same module. Either way, the loader state is wrong from startup onward and nothing in RED4ext will tell you so.

Suggested fix

Do not wrap GetModuleHandle in an owning RAII type. Either keep a raw HMODULE for the exe path (and use a tagged union or a separate "borrowed" path), or acquire a real owning reference with the API that is designed for that:

HMODULE raw = nullptr;
if (aPath.extension() == L".exe")
{
    if (!GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_PIN, nullptr, &raw))
    {
        ...
    }
}
else
{
    raw = LoadLibraryExW(aPath.c_str(), nullptr, flags);
}
wil::unique_hmodule handle(raw);

GET_MODULE_HANDLE_EX_FLAG_PIN would mean the process image is pinned for the lifetime of the process, which is fine because it cannot be unloaded anyway. Or rework the ownership model so the exe-path handle is non-owning.

Severity

High. Fires on every startup. Currently invisible because Windows is forgiving about the process image specifically, but this is a real Win32 contract violation and the kind of thing that bites years later when a new tool comes along that cares.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions