Repository navigation
1.7.8-beta.2: security and performance audit, Game Boost orchestration, installer changes - #75
Merged
Merged
Conversation
Game Boost now gives Windows hints as well as constraints, and adapts the game's CPU Set to its load instead of applying it once: - the game gets high QoS (power throttling off), on every CPU, so Windows never throttles it while it is not in the foreground; - in Isolation the listed background apps get EcoQoS, also on CPUs where CPU Sets are not applied; - a game that keeps its confined CPUs saturated has its CPU Set released so Windows can use every core, and confined again after a minute of low load (hysteresis, at most two releases per session). Same ownership rules as CPU Sets: only values Windows still manages are changed, user rules and the app's own QoS win, everything is journaled and restored on exit, toggle and after a crash. Efficiency mode writes now keep the process's other power-throttling flags: Windows replaces the whole state on set, so the old write wiped a timer-resolution opt-out set by the process itself.
The indeterminate ProgressRing kept WPF rendering every frame even when collapsed (~25% of a core with the window open, ~2% in the tray). It now animates only while visible. The SpinnerAnimation storyboard targeted a missing element, so its Begin() threw and the 15 s timeout guard was never created; both are removed.
…them to %TEMP% With IncludeNativeLibrariesForSelfExtract the elevated app loaded wpfgfx_cor3, PresentationNative_cor3 and D3DCompiler_47_cor3 from %TEMP%\.net\ThreadPilot, which the non-elevated user can modify. The installer already copies *.dll and the portable zip and smoke test take the whole publish folder.
… install ThreadPilot_Launch and ThreadPilot_Startup run with highest privileges and no prompt, yet were bound to whichever copy last ran elevated, including portable and dev folders any non-admin program can write to. Each elevated start now deletes ThreadPilot tasks that point outside Program Files and registers them only for an exe under Program Files, skipping the re-registration when the task already points at this exe. Enabling autostart from a portable copy shows why it is unavailable instead of a generic failure.
The elevated app wrote the scheduled-task definition and downloaded the update installer into the user's %TEMP%, where a non-admin process can swap them (or plant DLLs beside the installer) between the check and the elevated use. Both now go into a fresh, unguessable folder under %SystemRoot%\Temp whose DACL grants only Administrators and SYSTEM; users there cannot rename or delete another account's folders.
The installer runs elevated and silent, yet a release without the checksum file was installed unverified. The download now stops before fetching anything when the checksum asset is missing or not hosted on GitHub.
…ritable files Rules load from %AppData%, which any program of the user can edit, but only the priority path checked for protected processes; CPU Sets, memory and I/O priority and EcoQoS went straight through. The rules engine now checks the live process (critical-process list and PPL) before applying anything, and the list gains a few more system processes. Driver tweak restores accept only the values the tweak itself writes, and their backup file is written atomically.
A repointed third-party tag could swap the installer during the release job,
which held a write token for every job. Actions are now pinned to commit SHAs
(Dependabot keeps them current), only the job that publishes the release can
write, workflow inputs reach pwsh through env instead of ${{ }} interpolation,
Inno Setup and the SBOM tool are version-pinned, and the gitleaks download is
checked against its published SHA256.
Every 5 s refresh built a new FilteredProcesses collection, so the grid rebuilt all rows and dropped the sort picked from a column header. The filtered result is now synced into the existing collection with remove/move/insert, which keeps rows, selection and header sort (window-open idle CPU 3.0-3.5% -> 2.2% of a core).
- TieredCompilation off: the background tier-1 re-JIT cost ~4.5 s of CPU at every start for a tray app that gains little from it. Measured: startup CPU 8.8 s -> 5.5 s, idle after ~7 s instead of 9-18 s, same idle CPU, window +0.3 s. - The elevated launch task is ensured in the background: only the next launch needs it, and its schtasks calls held up the first window. - No second settings load in MainWindow (App already loaded them and applied the theme), no fixed 500 ms wait before hiding the overlay, and the cold WMI CPU-name query runs off the UI thread.
…owercfg - The Game Boost timer runs only while Game Boost is on (off by default); turning it off still restores everything through ReapplyAsync. - A foreground process just rejected as not-a-game is not rebuilt into a full process model every 3 s; it is looked at again after 30 s or on a settings change. - Switching power plans reads one /list for the previous and target plans and reuses it after /setactive: 3 powercfg.exe starts instead of 5 (2 when duplicate prevention is off).
- Registry priority now writes IFEO\<exe>\PerfOptions\CpuPriorityClass, the value Windows reads (the old PriorityClass value did nothing and is cleaned up), and refuses an empty image name that would have targeted the IFEO root. - Profile names must be plain file names, so they stay in the profiles folder. - P/Invoke DLLs load from System32 only; explorer.exe is started by full path. - The elevated restart passes arguments through ArgumentList (correct quoting). - The single-instance mutex no longer crashes startup when another session owns it. - Log size and retention are clamped and the size math is done in long. - BinaryFormatter is disabled: the app only copies text to the clipboard. - Two services registered but never resolved are no longer registered. - build/install-threadpilot.ps1 verifies the installer against SHA256SUMS.txt.
- The launch task is re-registered on every elevated start again (now in the background), so a changed definition or another admin never keeps a stale one. - Admin-only folders are owned by Administrators when elevated, so the user cannot rewrite their DACL with UAC off or a non-default owner policy. - The update provider trusts only folders it created itself. - Game Boost does not remember a foreground process as not-a-game while its path is still unreadable.
TieredCompilation=false saved the most startup CPU but cost about 1% of a core whenever the window is open (no tier-1 code). Turning off only dynamic PGO keeps most of the startup saving and the steady-state speed. Measured (interleaved, 2 rounds): startup CPU 7.8 s -> 6.1 s, window-open idle 2.4% (same as before), tray 0 ms over 90 s, window back to 1.3 s.
During a session the game was looked up twice every 3 s (identity + CPU time, each a GetProcessById and the identity a full process snapshot for SessionId), and every 15 s a model was built for every process on the machine to find a few listed background names. - The game is held open for the session (Process + Exited): its exit ends the session at once, under the same lock as the tick; CPU time for the load sampling is read from the same handle. The identity is re-checked through the held handle, which also pins the PID. - The handle is verified with one wait before the exit wait is registered, so a handle without SYNCHRONIZE (anti-cheat) falls into the polling fallback instead of failing on the runtime's wait thread. A denied or protected game keeps the previous polling. - The rescan filters the GetProcesses snapshot by listed name before any model is built; exact paths are still checked afterwards. - Session id read with ProcessIdToSessionId instead of Process.SessionId (a full snapshot per listed background process every 15 s). Ownership, journal and restore unchanged. +8 tests (903/903).
Process.EnableRaisingEvents opens the game with PROCESS_ALL_ACCESS and the session kept that handle for the whole game: exactly the kind of outside handle anti-cheats scan for, and a protected (PPL) game refused it and fell back to polling. The game is now opened with only SYNCHRONIZE and PROCESS_QUERY_LIMITED_INFORMATION; start time and CPU time come from GetProcessTimes on that handle, and the exit wait is a thread-pool registration on it, still checked with one zero wait first so a handle stripped of SYNCHRONIZE falls back to polling. +1 native test (a protected process can be held).
…nstalls always in Program Files - A portable copy that turns on autostart now offers to download, verify and install the official version through the update path (checksum required, admin-only folder); it never downgrades. The installed copy turns autostart on at its first start when the setting asks for it, after the stale-task cleanup, so it also self-heals a removed task. - The installer no longer asks for a folder: always Program Files, the only place a non-admin program cannot replace the elevated exe. Installs in another folder move there on upgrade; only ThreadPilot's own files are removed from the old folder (the old uninstaller is not run: it deletes the user's settings). Verified in Windows Sandbox: old install in C:\Apps moved to Program Files, user file and settings kept, the installed app registered both tasks itself.
…he free width In the wide layout the Priority column no longer stretches: the table is as wide as its visible columns (capped so the details keep at least 400 px) and the details panel gets the rest. A wide details panel shows Priority and CPU cores side by side, and the no-selection hint sits centered. The narrow layout (< 960 px) is unchanged.
App applied a fixed Normal state after MainWindow had restored the saved placement, so the saved maximized flag never took effect (since 1.7.7).
…e during a session ReapplyAsync put back the session's old process model, which still said AboveNormal after the restore had set the game back to Normal, so the priority was not applied again.
… a session Timer.Dispose does not wait for a callback already on the thread pool; such a tick could apply changes after the shutdown restore and leave the game handle open.
…them Stopping the process monitor untracks every process it changed, so the "reset on exit" step that ran afterwards always found nothing to reset (default-on setting, broken since before 1.7.7).
Application.Shutdown ignored the window's cancel, so Game Boost, reset-on-exit and the default power plan were never restored before the installer took over.
Windows does not start an admin-only exe from Run at logon, so the entry kept autostart off while the setting showed it on and the startup repair skipped creating the scheduled task.
A successful update exits right after starting the installer, so its ~55 MB folder was never removed. At elevated startup, folders older than a day are deleted, only when they are owned by Administrators (a user cannot create those or change what is inside).
The old folder outside Program Files may be user-writable; replaced by a junction, the admin deletes would land elsewhere. The old exe is deleted with retries (a copy still closing after an in-app update holds it), and the other files go only once the exe is gone.
On a preview build with prerelease updates off only the older stable release is found, and the user saw only the generic "needs Program Files" warning; network and download errors were dropped too. The reason is now added to the warning (new string in 7 languages).
Game Boost StopAsync and Dispose reuse SyncTimer for the timer; the always-true UpdateDownloadResult.ChecksumVerified and an unused using are gone. No behavior change.
An Auto grid column re-measured the DataGrid at infinite width on every list refresh: with the window open ThreadPilot used ~5.3% of a core instead of ~2.5% (ABBA A/B, 6 runs each). The width is now the sum of the visible columns, recomputed only when a column or the window changes; measured again at ~2.4%.
…PID for 30 s The memory of a rejected foreground process was keyed by PID only; it now also compares the process start time. Found by the independent Codex review.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 570750edb4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| this.isAutostartEnabled = taskRegistered || !string.IsNullOrWhiteSpace(legacyRegistryValue); | ||
| // Only the task starts an admin-only exe at logon; a leftover HKCU\Run entry does not. | ||
| this.isAutostartEnabled = taskRegistered; |
There was a problem hiding this comment.
When an installation still has only the old HKCU\...\Run value but the saved setting is off, this now reports autostart as disabled. The settings save path therefore sees false == false and never calls DisableAutostartAsync, leaving the legacy value to launch ThreadPilot at every logon (and potentially trigger its elevation bootstrap). Treat the legacy entry as needing cleanup, or remove it during status reconciliation.
Useful? React with 👍 / 👎.
| using (process) | ||
| { | ||
| // The name is already in the snapshot GetProcesses took, so skipping here costs no query. | ||
| if (nameFilter != null && !nameFilter(process.ProcessName)) |
There was a problem hiding this comment.
A process can exit after Process.GetProcesses() returns but before this property access; Process.ProcessName then throws, bypassing the existing exception handling in TryCreateProcessModel. This makes GetProcessesWithPathsAsync fault as a whole, so GameSessionService.ApplyToSessionAsync aborts that background rescan instead of applying Game Boost to any listed processes. Read the name under the same per-process exception handling (and skip unreadable/exited processes).
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security and performance audit, Game Boost orchestration and game handle, installer/portable changes, and the final review fixes for 1.7.8-beta.2. Full release notes:
docs/releases/v1.7.8-beta.2.md.Highlights
Verification
--smoke-testexit 0; ISCC build.C:\Apps(moved, user files and AppData kept), junction in place of the old folder (nothing deleted in the target), old copy still running (left in place).