Skip to content

[codex] Fall back when process path inspection fails#233

Draft
HavenDV wants to merge 1 commit into
masterfrom
codex/fix-188-process-path-fallback
Draft

[codex] Fall back when process path inspection fails#233
HavenDV wants to merge 1 commit into
masterfrom
codex/fix-188-process-path-fallback

Conversation

@HavenDV
Copy link
Copy Markdown
Owner

@HavenDV HavenDV commented Apr 11, 2026

Summary

  • stop treating Process.MainModule as a hard requirement when deriving the tray icon GUID
  • fall back to the existing assembly/AppContext path resolution when process inspection throws
  • keep the change limited to the shared process-path helper used by tray icon initialization

Why

On some systems, endpoint protection or memory-protection features cause Process.MainModule to throw while TaskbarIcon is initializing. That currently crashes the constructor path even though the library already has alternate ways to derive a stable process path.

Validation

  • dotnet build src/libs/H.NotifyIcon/H.NotifyIcon.csproj --configuration Release /p:GeneratePackageOnBuild=false
  • dotnet build src/libs/H.NotifyIcon.Wpf/H.NotifyIcon.Wpf.csproj --configuration Release /p:GeneratePackageOnBuild=false

Fixes #188

@claude
Copy link
Copy Markdown

claude Bot commented Apr 11, 2026

PR Review: Fall back when process path inspection fails

The change is small, focused, and correctly addresses the Sentinel 1 EDR crash reported in #188. A few observations:

What's good

  • The try/catch is tightly scoped — it wraps only the Process.GetCurrentProcess() block, so the fallback path (Assembly.GetEntryAssembly() / AppContext.BaseDirectory) remains reachable exactly as before.
  • The null check simplification (process?.MainModule?.FileNameprocess.MainModule?.FileName) is safe because GetCurrentProcess() never returns null; the ? was misleading noise.
  • The ! null-forgiving operator on return path! is correct: string.IsNullOrWhiteSpace already proves the string is non-null at that point.
  • Catching Win32Exception (the exception actually thrown by NtProcessManager) is the right exception type per the stack trace in CreateUniqueGuidForProcessPath() can fail on systems that have memory protection enabled #188.

Minor issues

1. Silent swallowing of InvalidOperationException — is it justified?

InvalidOperationException is the documented exception for Process.MainModule when the process has exited or the handle is invalid. That's legitimate to swallow here, but the comment says "process metadata is unavailable" — it might be worth briefly clarifying in the comment that this covers the process-has-exited case, so a future reader doesn't wonder if there's a broader class of InvalidOperationException being hidden.

catch (InvalidOperationException)
{
    // Process has exited or its module list is unavailable; fall through to assembly-based path.
}

2. AppContext.BaseDirectory is a directory, not a file path

This is a pre-existing issue, not introduced by this PR, but worth noting since the PR increases the likelihood of hitting the fallback:

location = AppContext.BaseDirectory;   // e.g. "C:\MyApp\" — a directory string

AppContext.BaseDirectory ends with a directory separator and points to the app directory, not the executable. CreateUniqueGuidFromString hashes whatever string is returned, so it will still produce a stable GUID, but two different applications deployed side-by-side in the same directory would collide. This might already be documented/accepted behaviour, but if the goal is a stable per-executable GUID it could be Path.Combine(AppContext.BaseDirectory, AppDomain.CurrentDomain.FriendlyName) or similar.

3. Assembly.GetEntryAssembly() throws if it returns null (pre-existing)

var assembly =
    Assembly.GetEntryAssembly() ??
    throw new InvalidOperationException("Entry assembly is not found.");

If both Process.MainModule and Assembly.GetEntryAssembly() are unavailable (e.g. hosted/unmanaged scenario), the library still throws. That was true before this PR, so it's not a regression, but it is now a more realistic path to hit given that endpoint security tools can interfere with process inspection.

Overall

The fix is correct and appropriate for the problem it solves. The two pre-existing edge cases above (#2 and #3) are out of scope for this PR — just something to track for a follow-up if users encounter them. The InvalidOperationException comment wording (#1) is a minor nit.

Verdict: Good to merge after addressing the comment wording nit (optional).

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.

CreateUniqueGuidForProcessPath() can fail on systems that have memory protection enabled

1 participant