Skip to content

fix(dart): Read Windows total physical memory via GlobalMemoryStatusEx instead of wmic.exe - #3894

Open
cf-rhett wants to merge 6 commits into
getsentry:mainfrom
cf-rhett:rhett/fix/windows-physical-memory-native
Open

fix(dart): Read Windows total physical memory via GlobalMemoryStatusEx instead of wmic.exe#3894
cf-rhett wants to merge 6 commits into
getsentry:mainfrom
cf-rhett:rhett/fix/windows-physical-memory-native

Conversation

@cf-rhett

@cf-rhett cf-rhett commented Jul 15, 2026

Copy link
Copy Markdown

📜 Description

On Windows, the pure-Dart IO device-context enricher (PlatformMemory) read TotalPhysicalMemory by shelling out to wmic.exe (wmic ComputerSystem get TotalPhysicalMemory /VALUE), falling back to powershell.exe when WMIC was absent. This replaces both process spawns with a direct kernel call to GlobalMemoryStatusEx from kernel32.dll via dart:ffi, reading MEMORYSTATUSEX.ullTotalPhys.

The returned value keeps the previous semantics (bytes), so device.memorySize is unchanged. The useWindowsWmci / useWindowsPowerShell probing, the WMIC/PowerShell helpers, and the wmic.exe/powershell.exe existence checks are all removed. Adds ffi: ^2.0.0 as a dependency — already used by sentry_flutter.

💡 Motivation and Context

WMIC is deprecated and being removed from current Windows installs. Two problems with the current approach:

  1. Endpoint-security noise. Every enriched event triggers a wmic.exe (or powershell.exe) child process under the host application. EDR/XDR tooling flags these spawns, and users ask why the app is invoking a deprecated Windows utility.
  2. Cost. Spawning a process to read a single integer is far heavier than a syscall — the caching added in [Flutter] startTransaction slows down grpc Requests on windows #2760 reduced how often it happens, but the spawn is still there on the first enriched event.

GlobalMemoryStatusEx is the documented Win32 API for this value, needs no child process, and works regardless of whether WMIC is installed. The library the current code was copied from (system_info2 / onepub-dev/system_info) still uses WMIC on its master branch, so there's no upstream version to lean on here.

💚 How did you test it?

Covered by the existing test/event_processor/enricher/io_platform_memory_test.dart, which asserts a non-null, positive TotalPhysicalMemory on Windows and Linux and null elsewhere. dart analyze and dart format are clean on the change.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes (existing test covers the Windows path)
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec
  • No breaking changes

Changelog Entry

Read Windows total physical memory via the native GlobalMemoryStatusEx API instead of spawning the deprecated wmic.exe (or PowerShell) process.

@cf-rhett
cf-rhett force-pushed the rhett/fix/windows-physical-memory-native branch from d5de069 to 26fc32f Compare July 15, 2026 14:33
@cf-rhett
cf-rhett marked this pull request as ready for review July 15, 2026 15:09
@codecov

codecov Bot commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 25.00000% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.06%. Comparing base (d527ac5) to head (8041659).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...c/event_processor/enricher/io_platform_memory.dart 25.00% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3894      +/-   ##
==========================================
+ Coverage   87.94%   88.06%   +0.11%     
==========================================
  Files         347      347              
  Lines       12929    12906      -23     
==========================================
- Hits        11371    11366       -5     
+ Misses       1558     1540      -18     
Flag Coverage Δ
sentry 87.86% <25.00%> (+0.23%) ⬆️
sentry_dio 97.73% <ø> (ø)
sentry_drift 93.57% <ø> (ø)
sentry_file 65.29% <ø> (ø)
sentry_firebase_remote_config 100.00% <ø> (ø)
sentry_flutter 92.16% <ø> (-0.01%) ⬇️
sentry_grpc 99.09% <ø> (ø)
sentry_hive 77.48% <ø> (ø)
sentry_isar 74.37% <ø> (ø)
sentry_link 21.50% <ø> (ø)
sentry_logging 97.01% <ø> (ø)
sentry_sqflite 88.81% <ø> (ø)
sentry_supabase 97.27% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Comment thread packages/dart/lib/src/event_processor/enricher/io_platform_memory.dart Outdated
The IO device-context enricher shelled out to `wmic.exe` (falling back to
PowerShell) to read `TotalPhysicalMemory` on Windows. WMIC is deprecated and
absent on recent Windows installs, and spawning a child process for every
enriched event is both slow and noisy for endpoint-security tooling that flags
`wmic.exe` executions.

Read the value directly from the kernel via `GlobalMemoryStatusEx`
(`kernel32.dll`) using `dart:ffi`, matching the previous byte-valued semantics
and removing the process spawn entirely. Adds the `ffi` package dependency,
already used by `sentry_flutter`.
@cf-rhett
cf-rhett force-pushed the rhett/fix/windows-physical-memory-native branch from 26fc32f to 6341cc4 Compare August 4, 2026 02:50
@lucas-zimerman lucas-zimerman changed the title fix: Read Windows total physical memory via GlobalMemoryStatusEx instead of wmic.exe fix(dart): Read Windows total physical memory via GlobalMemoryStatusEx instead of wmic.exe Aug 4, 2026

@lucas-zimerman lucas-zimerman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, CI passed with the changes in regard to windows, there are some failures on other platforms but probably unrelated to this PR.

@buenaflor Anything that you would like to point out? I checked locally, and the PR is working as intended, if there is nothing else to change from your part, LGTM!

@buenaflor

Copy link
Copy Markdown
Contributor

LGTM if you tested it as well. CI failures are because of missing auth for E2E tests, but that's fine

@buenaflor
buenaflor enabled auto-merge (squash) August 12, 2026 11:59
@buenaflor

Copy link
Copy Markdown
Contributor

There seems to be Analyze errors

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.

3 participants