From 04616cee162b5d2ad390aec34bf0289a426931f3 Mon Sep 17 00:00:00 2001 From: Valentin Radu Date: Thu, 11 Nov 2021 07:00:44 +0200 Subject: [PATCH] Fixes for shell extension * Properly implemented cleanup routines when injected into applications other than Explorer * Never EVER forget to specify a ThreadingModel for a COM object in registry. That was the cause the DLL was making Microsoft Teams crash on startup * Do not free memory when the DLL is unloaded (DLL_PROCESS_DETACH) and lpvReserved is non-NULL. It means the DLL is unloaded as the process terminates, in which case all threads are dead, so it is not safe to call freeing routines - let the operating system reclaim the memory on process exit --- CHANGELOG.md | 8 + ExplorerPatcher/ExplorerPatcher.rc | 8 +- ExplorerPatcher/SettingsMonitor.c | 152 +++++++++------- ExplorerPatcher/SettingsMonitor.h | 3 +- ExplorerPatcher/dllmain.c | 276 ++++++++++++++++++++--------- 5 files changed, 293 insertions(+), 154 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2aa5efc6a..3a147ae3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ This document includes the same release notes as in the [Releases](https://github.com/valinet/ExplorerPatcher/releases) section on GitHub. +## 22000.318.36 + +Tested on build 22000.318. + +#### Fixes + +* Lots of bug and issue fixes for shell extension failing to work under certain circumstances; fixed #259 + ## 22000.318.35 Tested on build 22000.318. diff --git a/ExplorerPatcher/ExplorerPatcher.rc b/ExplorerPatcher/ExplorerPatcher.rc index 1306fa123..a4bb01174 100644 --- a/ExplorerPatcher/ExplorerPatcher.rc +++ b/ExplorerPatcher/ExplorerPatcher.rc @@ -51,8 +51,8 @@ END // VS_VERSION_INFO VERSIONINFO - FILEVERSION 22000,318,35,0 - PRODUCTVERSION 22000,318,35,0 + FILEVERSION 22000,318,36,0 + PRODUCTVERSION 22000,318,36,0 FILEFLAGSMASK 0x3fL #ifdef _DEBUG FILEFLAGS 0x1L @@ -69,12 +69,12 @@ BEGIN BEGIN VALUE "CompanyName", "VALINET Solutions SRL" VALUE "FileDescription", "ExplorerPatcher" - VALUE "FileVersion", "22000.318.35.0" + VALUE "FileVersion", "22000.318.36.0" VALUE "InternalName", "ExplorerPatcher.dll" VALUE "LegalCopyright", "Copyright (C) 2006-2021 VALINET Solutions SRL. All rights reserved." VALUE "OriginalFilename", "ExplorerPatcher.dll" VALUE "ProductName", "ExplorerPatcher" - VALUE "ProductVersion", "22000.318.35.0" + VALUE "ProductVersion", "22000.318.36.0" END END BLOCK "VarFileInfo" diff --git a/ExplorerPatcher/SettingsMonitor.c b/ExplorerPatcher/SettingsMonitor.c index 0c477aaf6..5054e3776 100644 --- a/ExplorerPatcher/SettingsMonitor.c +++ b/ExplorerPatcher/SettingsMonitor.c @@ -3,83 +3,113 @@ DWORD WINAPI MonitorSettings(SettingsChangeParameters* params) { BOOL bShouldExit = FALSE; + HANDLE* handles = NULL; + printf("[SettingsMonitor] Started %p\n", params->settings[0].hEvent); while (TRUE) { - HANDLE* handles = malloc(sizeof(HANDLE) * (params->size + 1)); - if (!handles) + handles = calloc(sizeof(HANDLE), params->size); + if (handles) { - return 0; - } - for (unsigned int i = 0; i < params->size; ++i) - { - params->settings[i].hEvent = CreateEventW(NULL, FALSE, FALSE, NULL); - if (!params->settings[i].hEvent) - { - return 0; - } - handles[i] = params->settings[i].hEvent; - if (RegCreateKeyExW( - params->settings[i].origin, - params->settings[i].name, - 0, - NULL, - REG_OPTION_NON_VOLATILE, - KEY_READ, - NULL, - &(params->settings[i].hKey), - NULL - ) != ERROR_SUCCESS) + for (unsigned int i = 0; i < params->size; ++i) { - return 0; + if (i == 0) + { + if (params->settings[i].hEvent) + { + handles[i] = params->settings[i].hEvent; + continue; + } + else + { + InterlockedExchange(&(params->size), NULL); + return 0; + } + } + params->settings[i].hEvent = CreateEventW(NULL, FALSE, FALSE, NULL); + if (!params->settings[i].hEvent) + { + InterlockedExchange(&(params->size), 0); + return 0; + } + handles[i] = params->settings[i].hEvent; + if (RegCreateKeyExW( + params->settings[i].origin, + params->settings[i].name, + 0, + NULL, + REG_OPTION_NON_VOLATILE, + KEY_READ, + NULL, + &(params->settings[i].hKey), + NULL + ) != ERROR_SUCCESS) + { + InterlockedExchange(&(params->size), 0); + return 0; + } + if (RegNotifyChangeKeyValue( + params->settings[i].hKey, + FALSE, + REG_NOTIFY_CHANGE_LAST_SET, + params->settings[i].hEvent, + TRUE + ) != ERROR_SUCCESS) + { + InterlockedExchange(&(params->size), 0); + return 0; + } } - if (RegNotifyChangeKeyValue( - params->settings[i].hKey, + DWORD dwRes = WaitForMultipleObjects( + params->size, + handles, FALSE, - REG_NOTIFY_CHANGE_LAST_SET, - params->settings[i].hEvent, - TRUE - ) != ERROR_SUCCESS) - { - return 0; - } - } - handles[params->size] = params->hExitEvent; - DWORD dwRes = WaitForMultipleObjects( - params->size + (params->hExitEvent ? 1 : 0), - handles, - FALSE, - INFINITE - ); - if (dwRes != WAIT_FAILED) - { - unsigned int i = dwRes - WAIT_OBJECT_0; - if (i >= 0 && i < params->size) - { - params->settings[i].callback(params->settings[i].data); - } - else if (i == params->size && params->hExitEvent) + INFINITE + ); + if (dwRes != WAIT_FAILED) { - bShouldExit = TRUE; + unsigned int i = dwRes - WAIT_OBJECT_0; + if (i >= 1 && i < params->size) + { + params->settings[i].callback(params->settings[i].data); + } + else if (i == 0) + { + bShouldExit = TRUE; + } + for (unsigned int j = 1; j < params->size; ++j) + { + if (WaitForSingleObject(handles[j], 0) == WAIT_OBJECT_0) + { + params->settings[j].callback(params->settings[j].data); + } + } } - for (unsigned int j = 0; j < params->size; ++j) + free(handles); + for (unsigned int i = 1; i < params->size; ++i) { - if (WaitForSingleObject(handles[j], 0) == WAIT_OBJECT_0) + if (params->settings[i].hEvent) + { + CloseHandle(params->settings[i].hEvent); + } + if (params->settings[i].hKey) { - params->settings[j].callback(params->settings[j].data); + RegCloseKey(params->settings[i].hKey); } } + if (bShouldExit) + { + break; + } } - free(handles); - for (unsigned int i = 0; i < params->size; ++i) - { - CloseHandle(params->settings[i].hEvent); - RegCloseKey(params->settings[i].hKey); - } - if (bShouldExit) + else { - break; + InterlockedExchange(&(params->size), 0); + return 0; } } + printf("[SettingsMonitor] Ended %p\n", params->settings[0].hEvent); + InterlockedExchange(&(params->size), 0); + return 0; } diff --git a/ExplorerPatcher/SettingsMonitor.h b/ExplorerPatcher/SettingsMonitor.h index 8a57157ae..a80a635b8 100644 --- a/ExplorerPatcher/SettingsMonitor.h +++ b/ExplorerPatcher/SettingsMonitor.h @@ -3,6 +3,7 @@ #include #include #pragma comment(lib, "Shlwapi.lib") +#include typedef struct _Setting { @@ -17,7 +18,7 @@ typedef struct _SettingsChangeParameters { Setting* settings; DWORD size; - HANDLE hExitEvent; + HANDLE hThread; } SettingsChangeParameters; DWORD WINAPI MonitorSettings(SettingsChangeParameters*); #endif diff --git a/ExplorerPatcher/dllmain.c b/ExplorerPatcher/dllmain.c index 6fe6a6ae5..0c7621bcd 100644 --- a/ExplorerPatcher/dllmain.c +++ b/ExplorerPatcher/dllmain.c @@ -70,11 +70,9 @@ DWORD bTaskbarMonitorOverride = 0; DWORD dwIMEStyle = 0; DWORD dwTaskbarAl = 0; HMODULE hModule = NULL; -HANDLE hSettingsMonitorThread = NULL; HANDLE hDelayedInjectionThread = NULL; HANDLE hIsWinXShown = NULL; HANDLE hWinXThread = NULL; -HANDLE hExitSettingsMonitor = NULL; HANDLE hSwsSettingsChanged = NULL; HANDLE hSwsOpacityMaybeChanged = NULL; BYTE* lpShouldDisplayCCButton = NULL; @@ -3948,92 +3946,159 @@ __declspec(dllexport) DWORD WINAPI main( hSwsOpacityMaybeChanged = CreateEventW(NULL, FALSE, FALSE, NULL); } - settings = calloc(10, sizeof(Setting)); - settings[0].callback = LoadSettings; - settings[0].data = bIsExplorer; - settings[0].hEvent = NULL; - settings[0].hKey = NULL; - wcscpy_s(settings[0].name, MAX_PATH, TEXT(REGPATH)); - settings[0].origin = HKEY_CURRENT_USER; - - settings[1].callback = LoadSettings; - settings[1].data = bIsExplorer; - settings[1].hEvent = NULL; - settings[1].hKey = NULL; - wcscpy_s(settings[1].name, MAX_PATH, L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Explorer\\StartPage"); - settings[1].origin = HKEY_CURRENT_USER; - - settings[2].callback = SetEvent; - settings[2].data = hSwsSettingsChanged; - settings[2].hEvent = NULL; - settings[2].hKey = NULL; - wcscpy_s(settings[2].name, MAX_PATH, TEXT(REGPATH) L"\\sws"); - settings[2].origin = HKEY_CURRENT_USER; - - settings[3].callback = SetEvent; - settings[3].data = hSwsOpacityMaybeChanged; - settings[3].hEvent = NULL; - settings[3].hKey = NULL; - wcscpy_s(settings[3].name, MAX_PATH, L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Explorer\\MultitaskingView\\AltTabViewHost"); - settings[3].origin = HKEY_CURRENT_USER; - - settings[4].callback = Explorer_RefreshUI; - settings[4].data = NULL; - settings[4].hEvent = NULL; - settings[4].hKey = NULL; - wcscpy_s(settings[4].name, MAX_PATH, L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Explorer\\Advanced"); - settings[4].origin = HKEY_CURRENT_USER; - - settings[5].callback = Explorer_RefreshUI; - settings[5].data = NULL; - settings[5].hEvent = NULL; - settings[5].hKey = NULL; - wcscpy_s(settings[5].name, MAX_PATH, L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Search"); - settings[5].origin = HKEY_CURRENT_USER; - - settings[6].callback = Explorer_RefreshUI; - settings[6].data = NULL; - settings[6].hEvent = NULL; - settings[6].hKey = NULL; - wcscpy_s(settings[6].name, MAX_PATH, L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Explorer\\Advanced\\People"); - settings[6].origin = HKEY_CURRENT_USER; - - settings[7].callback = Explorer_RefreshUI; - settings[7].data = NULL; - settings[7].hEvent = NULL; - settings[7].hKey = NULL; - wcscpy_s(settings[7].name, MAX_PATH, L"SOFTWARE\\Microsoft\\TabletTip\\1.7"); - settings[7].origin = HKEY_CURRENT_USER; - - settings[8].callback = SetEvent; - settings[8].data = hSwsSettingsChanged; - settings[8].hEvent = NULL; - settings[8].hKey = NULL; - wcscpy_s(settings[8].name, MAX_PATH, L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Explorer"); - settings[8].origin = HKEY_CURRENT_USER; - - settings[9].callback = UpdateStartMenuPositioning; - settings[9].data = MAKELPARAM(FALSE, TRUE); - settings[9].hEvent = NULL; - settings[9].hKey = NULL; - wcscpy_s(settings[9].name, MAX_PATH, L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Explorer\\Advanced"); - settings[9].origin = HKEY_CURRENT_USER; - - settingsParams = calloc(1, sizeof(SettingsChangeParameters)); - settingsParams->settings = settings; - settingsParams->size = bIsExplorer ? 10 : 1; - hExitSettingsMonitor = CreateEventW(NULL, FALSE, FALSE, NULL); - settingsParams->hExitEvent = hExitSettingsMonitor; - if (!hSettingsMonitorThread) - { - hSettingsMonitorThread = CreateThread( - 0, - 0, - MonitorSettings, - settingsParams, - 0, - 0 - ); + if (!settings && !settingsParams) + { + unsigned int numSettings = bIsExplorer ? 11 : 2; + settings = calloc(numSettings, sizeof(Setting)); + if (settings) + { + unsigned int cs = 0; + + if (cs < numSettings) + { + settings[cs].callback = NULL; + settings[cs].data = NULL; + settings[cs].hEvent = CreateEventW(NULL, FALSE, FALSE, NULL); + settings[cs].hKey = NULL; + ZeroMemory(settings[cs].name, MAX_PATH); + settings[cs].origin = NULL; + cs++; + } + + if (cs < numSettings) + { + settings[cs].callback = LoadSettings; + settings[cs].data = bIsExplorer; + settings[cs].hEvent = NULL; + settings[cs].hKey = NULL; + wcscpy_s(settings[cs].name, MAX_PATH, TEXT(REGPATH)); + settings[cs].origin = HKEY_CURRENT_USER; + cs++; + } + + if (cs < numSettings) + { + settings[cs].callback = LoadSettings; + settings[cs].data = bIsExplorer; + settings[cs].hEvent = NULL; + settings[cs].hKey = NULL; + wcscpy_s(settings[cs].name, MAX_PATH, L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Explorer\\StartPage"); + settings[cs].origin = HKEY_CURRENT_USER; + cs++; + } + + if (cs < numSettings) + { + settings[cs].callback = SetEvent; + settings[cs].data = hSwsSettingsChanged; + settings[cs].hEvent = NULL; + settings[cs].hKey = NULL; + wcscpy_s(settings[cs].name, MAX_PATH, TEXT(REGPATH) L"\\sws"); + settings[cs].origin = HKEY_CURRENT_USER; + cs++; + } + + if (cs < numSettings) + { + settings[cs].callback = SetEvent; + settings[cs].data = hSwsOpacityMaybeChanged; + settings[cs].hEvent = NULL; + settings[cs].hKey = NULL; + wcscpy_s(settings[cs].name, MAX_PATH, L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Explorer\\MultitaskingView\\AltTabViewHost"); + settings[cs].origin = HKEY_CURRENT_USER; + cs++; + } + + if (cs < numSettings) + { + settings[cs].callback = Explorer_RefreshUI; + settings[cs].data = NULL; + settings[cs].hEvent = NULL; + settings[cs].hKey = NULL; + wcscpy_s(settings[cs].name, MAX_PATH, L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Explorer\\Advanced"); + settings[cs].origin = HKEY_CURRENT_USER; + cs++; + } + + if (cs < numSettings) + { + settings[cs].callback = Explorer_RefreshUI; + settings[cs].data = NULL; + settings[cs].hEvent = NULL; + settings[cs].hKey = NULL; + wcscpy_s(settings[cs].name, MAX_PATH, L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Search"); + settings[cs].origin = HKEY_CURRENT_USER; + cs++; + } + + if (cs < numSettings) + { + settings[cs].callback = Explorer_RefreshUI; + settings[cs].data = NULL; + settings[cs].hEvent = NULL; + settings[cs].hKey = NULL; + wcscpy_s(settings[cs].name, MAX_PATH, L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Explorer\\Advanced\\People"); + settings[cs].origin = HKEY_CURRENT_USER; + cs++; + } + + if (cs < numSettings) + { + settings[cs].callback = Explorer_RefreshUI; + settings[cs].data = NULL; + settings[cs].hEvent = NULL; + settings[cs].hKey = NULL; + wcscpy_s(settings[cs].name, MAX_PATH, L"SOFTWARE\\Microsoft\\TabletTip\\1.7"); + settings[cs].origin = HKEY_CURRENT_USER; + cs++; + } + + if (cs < numSettings) + { + settings[cs].callback = SetEvent; + settings[cs].data = hSwsSettingsChanged; + settings[cs].hEvent = NULL; + settings[cs].hKey = NULL; + wcscpy_s(settings[cs].name, MAX_PATH, L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Explorer"); + settings[cs].origin = HKEY_CURRENT_USER; + cs++; + } + + if (cs < numSettings) + { + settings[cs].callback = UpdateStartMenuPositioning; + settings[cs].data = MAKELPARAM(FALSE, TRUE); + settings[cs].hEvent = NULL; + settings[cs].hKey = NULL; + wcscpy_s(settings[cs].name, MAX_PATH, L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Explorer\\Advanced"); + settings[cs].origin = HKEY_CURRENT_USER; + cs++; + } + + settingsParams = calloc(1, sizeof(SettingsChangeParameters)); + if (settingsParams) + { + settingsParams->settings = settings; + InterlockedExchange(&(settingsParams->size), numSettings); + settingsParams->hThread = CreateThread( + 0, + 0, + MonitorSettings, + settingsParams, + 0, + 0 + ); + } + else + { + if (numSettings && settings[0].hEvent) + { + CloseHandle(settings[0].hEvent); + } + free(settings); + settings = NULL; + } + } } InjectBasicFunctions(bIsExplorer, TRUE); @@ -4699,6 +4764,14 @@ HRESULT WINAPI _DllRegisterServer() wszFilename, (wcslen(wszFilename) + 1) * sizeof(wchar_t) ); + dwLastError = RegSetValueExW( + hKey, + L"ThreadingModel", + 0, + REG_SZ, + L"Apartment", + 10 * sizeof(wchar_t) + ); RegCloseKey(hKey); } } @@ -4735,6 +4808,14 @@ HRESULT WINAPI _DllRegisterServer() wszFilename, (wcslen(wszFilename) + 1) * sizeof(wchar_t) ); + dwLastError = RegSetValueExW( + hKey, + L"ThreadingModel", + 0, + REG_SZ, + L"Apartment", + 10 * sizeof(wchar_t) + ); RegCloseKey(hKey); } } @@ -5213,6 +5294,25 @@ BOOL WINAPI DllMain( case DLL_THREAD_DETACH: break; case DLL_PROCESS_DETACH: + if (!lpvReserved && bInstanced) + { + if (settings && settingsParams) + { + SetEvent(settings[0].hEvent); + if (WaitForSingleObject(settingsParams->hThread, 0) != WAIT_OBJECT_0) + { + while (InterlockedCompareExchange(&(settingsParams->size), 0, 0)) {}; + } + CloseHandle(settings[0].hEvent); + CloseHandle(settingsParams->hThread); + free(settingsParams); + settingsParams = NULL; + free(settings); + settings = NULL; + } + InjectBasicFunctions(FALSE, FALSE); + bInstanced = FALSE; + } break; } return TRUE;