Skip to content

Fix Windows crash saving tool definitions to a read-only install dir#5

Merged
stytim merged 1 commit into
mainfrom
fix/windows-data-dir
Jun 13, 2026
Merged

Fix Windows crash saving tool definitions to a read-only install dir#5
stytim merged 1 commit into
mainfrom
fix/windows-data-dir

Conversation

@stytim

@stytim stytim commented Jun 13, 2026

Copy link
Copy Markdown
Owner

Problem

On Windows, clicking Add Tool Definition crashed the app:

No RealSense devices found.
Added Tool1 with 4 spheres
An error occurred: create_directories: Access is denied.: ".\Tools"

The reporter hit this with no camera attached, but the save path is device-independent.

Root cause

GetDataDirectory() returned "." on Windows, so tool definitions (and CSV recordings) were written relative to the process working directory. For an app installed under C:\Program Files\… that directory is read-only, so fs::create_directories(".\Tools") failed with Access is denied. Because SaveToolDefinition() used the throwing create_directories overload, the filesystem_error propagated up to main() and terminated the app.

This is the same class of bug already fixed for macOS (where the .app working directory is /); Windows/Linux had been left on ".".

Fix

  • GetDataDirectory() now returns %LOCALAPPDATA%\IR Tracking App on Windows — a writable per-user location — mirroring the macOS ~/Library/Application Support handling. Uses _wdupenv_s (not getenv) to stay warning-clean under /W4 and to preserve non-ASCII user names. Refactored to a single create/log/return path. Linux still falls back to ".".
  • SaveToolDefinition() now uses the non-throwing create_directories(dir, ec) overload (and drops the throwing fs::exists): on failure it logs and skips persistence instead of crashing. The tool remains active for the session even if it can't be saved to disk.

Testing

  • macOS: incremental build clean, no new warnings.
  • Windows: the _WIN32 branch is compiled by this PR's build.yml / windows-2022 job (it couldn't be compiled on the macOS dev machine).

Notes

  • Data location moves on Windows from the (broken, read-only) install dir to %LOCALAPPDATA%. No migration needed — the old location never worked for installed users, so there's nothing to lose.
  • Two same-class but now-unreachable throwing filesystem sites remain (LoadToolDefinition, and the fs::exists loop in WriteToCSV on csvThread). Left out to keep this hotfix focused; happy to harden them in a follow-up.

Targets a v1.3.1 patch release over v1.3.0.

On Windows GetDataDirectory() returned ".", so tool definitions and CSV
recordings were written relative to the working directory. For an app
installed under C:\Program Files\ that directory is read-only, so
create_directories() failed; SaveToolDefinition() used the throwing
overload, so the filesystem_error propagated to main() and terminated the
app when clicking "Add Tool Definition" (reported with no device attached,
but the save path is device-independent).

- GetDataDirectory(): use %LOCALAPPDATA%\IR Tracking App on Windows, a
  writable per-user location, mirroring the macOS Application Support
  handling. _wdupenv_s avoids the MSVC getenv deprecation warning under /W4
  and preserves non-ASCII user names.
- SaveToolDefinition(): use the non-throwing create_directories overload;
  log and skip persistence on failure instead of crashing. The tool stays
  active for the session even if it can't be written to disk.
@stytim stytim merged commit a2adb76 into main Jun 13, 2026
3 checks passed
@stytim stytim deleted the fix/windows-data-dir branch June 13, 2026 20:56
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.

1 participant