Skip to content

Add This PC and Desktop to Nav Top: defer injection to avoid early conflicts#4282

Open
rodboev wants to merge 8 commits into
ramensoftware:mainfrom
rodboev:defer-injection
Open

Add This PC and Desktop to Nav Top: defer injection to avoid early conflicts#4282
rodboev wants to merge 8 commits into
ramensoftware:mainfrom
rodboev:defer-injection

Conversation

@rodboev
Copy link
Copy Markdown
Contributor

@rodboev rodboev commented Jun 3, 2026

Changed the injection strategy to hook kernelbase!LoadLibraryExW instead of force-loading ExplorerFrame.dll in Wh_ModInit. Processes that don't use ExplorerFrame.dll no longer get it loaded at startup.

A user reported instability on 25H2 in #4280 (browser tabs crashing, Explorer becoming unresponsive after restart). I haven't been able to reproduce it, but the previous approach loaded ExplorerFrame.dll into every injected process at startup, which is unnecessarily aggressive. Deferring to the kernelbase hook (per @m417z's suggestion in #4159) avoids that entirely.

@m417z
Copy link
Copy Markdown
Member

m417z commented Jun 4, 2026

Submission review

Note: This review was done by Claude, and then refined manually. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

I'm not sure if ExplorerFrame.dll is ever unloaded, but the concern makes sense and the fix is easy.


Removing the force-load also removed the implicit pin on ExplorerFrame.dll — consider pinning it explicitly when you hook it late. In the old code, the LoadLibraryExW(L"ExplorerFrame.dll", ...) fallback added a reference that was never released, so once hooked, the module stayed resident for the process lifetime. The new code uses the caller's handle in CheckLateLoadExplorerFrame without taking a reference, while g_explorerFrameLoaded latches true permanently. So if a process loads ExplorerFrame.dll transiently — e.g. an app that opens a modern file dialog and later dismisses it, and the module's refcount drops to 0 — the inline hooks are left pointing into an unmapped image, and because the flag stays true, the next dialog's reload is never re-hooked: the feature silently stops working for the rest of that process's life (and a reload at a different base is a stability risk). This won't bite explorer.exe (ExplorerFrame stays loaded there), but it's exactly the non-shell-process case @include * exists to cover.

The fix keeps the per-process pin you actually need without reintroducing the system-wide preload you're removing — pin it at the point you commit to hooking, in CheckLateLoadExplorerFrame (and equally in the already-loaded branch of Wh_ModInit):

// keep ExplorerFrame.dll resident for as long as our hooks live in it
HMODULE pin = nullptr;
GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_PIN, L"ExplorerFrame.dll", &pin);
Optional improvements

Minor polish — none of this affects users, so it's your call.

  • The else fallback to the kernel32-linked LoadLibraryExW is effectively dead code. kernelbase.dll is loaded in every Win32 process, so GetModuleHandleW(L"kernelbase.dll") + GetProcAddress never fail in practice; the fallback would only ever run in an impossible state, and if it did it would hook the wrong copy (the exact thing this PR fixes). Harmless as a belt-and-suspenders guard, but you could drop it (or leave a one-line comment noting it's unreachable) to make the intent clearer.

@rodboev
Copy link
Copy Markdown
Contributor Author

rodboev commented Jun 4, 2026

Got it. ExplorerFrame.dll is now pinned via GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_PIN) in both the late-load and already-loaded paths, and the unreachable kernelbase fallback is removed.

Also addressing #4293 by preventing the tree selection from shifting when the mod inserts or removes items. This was causing navigation in Explorer. Genuine user navigation is unaffected. This really speeds things up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants