From f64a7483da6052eaf5b7c42a0643b8508b371a4c Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Thu, 17 Sep 2026 20:07:12 -0500 Subject: [PATCH 01/23] Add periodic memory leak reporting Track Windows and Linux leak counts without making the known baseline block unrelated changes. Pin and verify Dr. Memory, retain raw reports, and publish per-scenario summaries for unit tests, functional tests, and SampleCppMini. Files changed: - .github/workflows/memory-leak-analysis.yml - .github/scripts/run-drmemory.ps1 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- .github/scripts/run-drmemory.ps1 | 108 +++++++++++++ .github/workflows/memory-leak-analysis.yml | 169 +++++++++++++++++++++ 2 files changed, 277 insertions(+) create mode 100644 .github/scripts/run-drmemory.ps1 create mode 100644 .github/workflows/memory-leak-analysis.yml diff --git a/.github/scripts/run-drmemory.ps1 b/.github/scripts/run-drmemory.ps1 new file mode 100644 index 000000000..324baf3ad --- /dev/null +++ b/.github/scripts/run-drmemory.ps1 @@ -0,0 +1,108 @@ +[CmdletBinding()] +param( + [Parameter(Mandatory = $true)] + [ValidateNotNullOrEmpty()] + [string]$DrMemoryPath, + + [Parameter(Mandatory = $true)] + [ValidateNotNullOrEmpty()] + [string]$LogDirectory, + + [Parameter(Mandatory = $true)] + [ValidatePattern('^[A-Za-z0-9_.-]+$')] + [string]$Scenario, + + [Parameter(Mandatory = $true)] + [ValidateNotNullOrEmpty()] + [string]$TargetPath, + + [string[]]$TargetArguments = @() +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = "Stop" + +function Get-LeakCount { + param( + [Parameter(Mandatory = $true)] + [string]$Results, + + [Parameter(Mandatory = $true)] + [string]$Category + ) + + $escapedCategory = [regex]::Escape($Category) + $pattern = "(?m)^\s*~~Dr\.M~~\s+([\d,]+) unique,\s+([\d,]+) total,\s+([\d,]+) byte\(s\) of $escapedCategory\r?$" + $match = [regex]::Match($Results, $pattern) + if (-not $match.Success) { + throw "Dr. Memory results do not contain the '$Category' summary." + } + + return @{ + Unique = [int64]($match.Groups[1].Value -replace ",", "") + Total = [int64]($match.Groups[2].Value -replace ",", "") + Bytes = [int64]($match.Groups[3].Value -replace ",", "") + } +} + +$resolvedDrMemoryPath = (Resolve-Path -LiteralPath $DrMemoryPath).Path +$resolvedTargetPath = (Resolve-Path -LiteralPath $TargetPath).Path +$resolvedLogDirectory = [System.IO.Path]::GetFullPath($LogDirectory) +$scenarioDirectory = Join-Path $resolvedLogDirectory $Scenario +New-Item -ItemType Directory -Path $scenarioDirectory -Force | Out-Null + +Write-Host "Running Dr. Memory leak analysis for $Scenario" +& $resolvedDrMemoryPath ` + -batch ` + -leaks_only ` + -logdir $scenarioDirectory ` + -- ` + $resolvedTargetPath ` + @TargetArguments +$targetExitCode = $LASTEXITCODE +if ($targetExitCode -ne 0) { + throw "Dr. Memory or $Scenario exited with code $targetExitCode." +} + +$resultFiles = @(Get-ChildItem -LiteralPath $scenarioDirectory -Filter results.txt -File -Recurse) +if ($resultFiles.Count -ne 1) { + throw "Expected one Dr. Memory results.txt for $Scenario, found $($resultFiles.Count)." +} + +$results = Get-Content -LiteralPath $resultFiles[0].FullName -Raw +$leaks = Get-LeakCount -Results $results -Category "leak(s)" +$possibleLeaks = Get-LeakCount -Results $results -Category "possible leak(s)" +$reachable = Get-LeakCount -Results $results -Category "still-reachable allocation(s)" + +$summary = [pscustomobject]@{ + Platform = if ($env:RUNNER_OS) { $env:RUNNER_OS } else { [System.Environment]::OSVersion.Platform } + Scenario = $Scenario + UniqueLeaks = $leaks.Unique + TotalLeaks = $leaks.Total + LeakBytes = $leaks.Bytes + UniquePossibleLeaks = $possibleLeaks.Unique + TotalPossibleLeaks = $possibleLeaks.Total + PossibleLeakBytes = $possibleLeaks.Bytes + UniqueReachable = $reachable.Unique + TotalReachable = $reachable.Total + ReachableBytes = $reachable.Bytes +} + +$summaryPath = Join-Path $resolvedLogDirectory "summary.csv" +$summaries = if (Test-Path -LiteralPath $summaryPath) { + @(Import-Csv -LiteralPath $summaryPath) + @($summary) +} +else { + @($summary) +} +$summaries | Export-Csv -LiteralPath $summaryPath -NoTypeInformation + +$markdown = @" +| Scenario | Unique leaks | Total leaks | Leak bytes | Unique possible | Possible bytes | Unique reachable | Reachable bytes | +|---|---:|---:|---:|---:|---:|---:|---:| +| $Scenario | $($leaks.Unique) | $($leaks.Total) | $($leaks.Bytes) | $($possibleLeaks.Unique) | $($possibleLeaks.Bytes) | $($reachable.Unique) | $($reachable.Bytes) | +"@ +Write-Host $markdown +if ($env:GITHUB_STEP_SUMMARY) { + Add-Content -LiteralPath $env:GITHUB_STEP_SUMMARY -Value $markdown +} diff --git a/.github/workflows/memory-leak-analysis.yml b/.github/workflows/memory-leak-analysis.yml new file mode 100644 index 000000000..f7dafa1b1 --- /dev/null +++ b/.github/workflows/memory-leak-analysis.yml @@ -0,0 +1,169 @@ +name: Periodic memory leak analysis + +on: + workflow_dispatch: + schedule: + - cron: 0 5 * * 1 + +permissions: + contents: read + +concurrency: + group: memory-leak-analysis-${{ github.ref }} + cancel-in-progress: false + +env: + DRMEMORY_VERSION: 2.6.20434 + DRMEMORY_TAG: cronbuild-2.6.20434 + +jobs: + windows: + name: Dr. Memory on Windows + runs-on: windows-2022 + timeout-minutes: 120 + steps: + - name: Checkout + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + + - name: Initialize googletest + run: git submodule update --init --depth=1 third_party/googletest + + - name: Setup MSBuild + uses: microsoft/setup-msbuild@6fb02220983dee41ce7ae257b6f4d8f9bf5ed4ce # v2.0.0 + with: + vs-version: '[17,)' + + - name: Build leak-analysis targets + shell: cmd + run: >- + tools\gen-version.cmd && + msbuild Solutions\MSTelemetrySDK.sln + /target:sqlite:Rebuild,zlib:Rebuild,Tests\gmock:Rebuild,Tests\gtest:Rebuild,Tests\UnitTests:Rebuild,Tests\FuncTests:Rebuild,Samples\cpp\SampleCppMini:Rebuild + /p:BuildProjectReferences=true + /p:Configuration=Debug + /p:Platform=x64 + /p:MATSDK_USE_WININET=false + /maxcpucount:2 + + - name: Download Dr. Memory + shell: pwsh + env: + DRMEMORY_SHA256: ED9C0E3F1BDB7F8DB1ADC13531493FB1C451E15F375638592C01D8837825A73A + run: | + $archive = Join-Path $env:RUNNER_TEMP "DrMemory-Windows-$env:DRMEMORY_VERSION.zip" + $url = "https://github.com/DynamoRIO/drmemory/releases/download/$env:DRMEMORY_TAG/DrMemory-Windows-$env:DRMEMORY_VERSION.zip" + Invoke-WebRequest -Uri $url -OutFile $archive + $actualHash = (Get-FileHash -LiteralPath $archive -Algorithm SHA256).Hash + if ($actualHash -ne $env:DRMEMORY_SHA256) { + throw "Dr. Memory archive hash mismatch: expected $env:DRMEMORY_SHA256, got $actualHash." + } + Expand-Archive -LiteralPath $archive -DestinationPath $env:RUNNER_TEMP + + - name: Analyze unit tests + shell: pwsh + run: >- + ./.github/scripts/run-drmemory.ps1 + -DrMemoryPath "$env:RUNNER_TEMP/DrMemory-Windows-$env:DRMEMORY_VERSION/bin64/drmemory.exe" + -LogDirectory drmemory-results + -Scenario unit-tests + -TargetPath Solutions/out/Debug/x64/UnitTests/UnitTests.exe + + - name: Analyze functional tests + shell: pwsh + run: >- + ./.github/scripts/run-drmemory.ps1 + -DrMemoryPath "$env:RUNNER_TEMP/DrMemory-Windows-$env:DRMEMORY_VERSION/bin64/drmemory.exe" + -LogDirectory drmemory-results + -Scenario functional-tests + -TargetPath Solutions/out/Debug/x64/FuncTests/FuncTests.exe + + - name: Analyze basic sample + shell: pwsh + run: >- + ./.github/scripts/run-drmemory.ps1 + -DrMemoryPath "$env:RUNNER_TEMP/DrMemory-Windows-$env:DRMEMORY_VERSION/bin64/drmemory.exe" + -LogDirectory drmemory-results + -Scenario sample-cpp-mini + -TargetPath Solutions/out/Debug/x64/SampleCppMini/SampleCppMini.exe + + - name: Upload Windows reports + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: drmemory-windows + path: drmemory-results + if-no-files-found: error + retention-days: 90 + + linux: + name: Dr. Memory on Linux + runs-on: ubuntu-22.04 + timeout-minutes: 120 + env: + CMAKE_POLICY_VERSION_MINIMUM: "3.5" + steps: + - name: Checkout + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + + - name: Initialize googletest + run: git submodule update --init --depth=1 third_party/googletest + + - name: Build leak-analysis targets + env: + CMAKE_OPTS: -DMATSDK_BUILD_UNIT_TESTS=ON -DMATSDK_BUILD_FUNC_TESTS=ON + run: ./build.sh debug + + - name: Build basic sample + run: | + cmake --install out --prefix "$PWD/out/install" + cmake -S examples/cpp/SampleCppMini -B out/sample-cpp-mini \ + -DCMAKE_BUILD_TYPE=Debug \ + -DMATSDK_INSTALL_DIR="$PWD/out/install" + cmake --build out/sample-cpp-mini --parallel 2 + + - name: Download Dr. Memory + env: + DRMEMORY_SHA256: 79B7718C0040A68B4FCECD9BA1C422174350A5B3A40A8417047A9219E9C2F258 + run: | + archive="$RUNNER_TEMP/DrMemory-Linux-$DRMEMORY_VERSION.tar.gz" + url="https://github.com/DynamoRIO/drmemory/releases/download/$DRMEMORY_TAG/DrMemory-Linux-$DRMEMORY_VERSION.tar.gz" + curl --fail --location --retry 3 --output "$archive" "$url" + echo "$DRMEMORY_SHA256 $archive" | sha256sum --check --strict + tar -xzf "$archive" -C "$RUNNER_TEMP" + + - name: Analyze unit tests + shell: pwsh + run: >- + ./.github/scripts/run-drmemory.ps1 + -DrMemoryPath "$env:RUNNER_TEMP/DrMemory-Linux-$env:DRMEMORY_VERSION/bin64/drmemory" + -LogDirectory drmemory-results + -Scenario unit-tests + -TargetPath out/tests/unittests/UnitTests + + - name: Analyze functional tests + shell: pwsh + run: >- + ./.github/scripts/run-drmemory.ps1 + -DrMemoryPath "$env:RUNNER_TEMP/DrMemory-Linux-$env:DRMEMORY_VERSION/bin64/drmemory" + -LogDirectory drmemory-results + -Scenario functional-tests + -TargetPath out/tests/functests/FuncTests + -TargetArguments "--gtest_filter=-APITest.C_API_Test" + + - name: Analyze basic sample + shell: pwsh + run: >- + ./.github/scripts/run-drmemory.ps1 + -DrMemoryPath "$env:RUNNER_TEMP/DrMemory-Linux-$env:DRMEMORY_VERSION/bin64/drmemory" + -LogDirectory drmemory-results + -Scenario sample-cpp-mini + -TargetPath out/sample-cpp-mini/SampleCppMini + + - name: Upload Linux reports + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: drmemory-linux + path: drmemory-results + if-no-files-found: error + retention-days: 90 From c64c35a17d10fbe06db8f8af15b67f5a8c377afa Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Thu, 17 Sep 2026 20:07:40 -0500 Subject: [PATCH 02/23] Validate leak workflow changes on pull requests Run the expensive analysis only when its workflow or helper changes, so this PR and future maintenance updates exercise both hosted platforms before merge. Files changed: - .github/workflows/memory-leak-analysis.yml Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- .github/workflows/memory-leak-analysis.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/memory-leak-analysis.yml b/.github/workflows/memory-leak-analysis.yml index f7dafa1b1..0ec2ba3ab 100644 --- a/.github/workflows/memory-leak-analysis.yml +++ b/.github/workflows/memory-leak-analysis.yml @@ -2,6 +2,12 @@ name: Periodic memory leak analysis on: workflow_dispatch: + pull_request: + branches: + - main + paths: + - .github/scripts/run-drmemory.ps1 + - .github/workflows/memory-leak-analysis.yml schedule: - cron: 0 5 * * 1 From c12d72595bf93d50107554debffc735d68667cc7 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Thu, 17 Sep 2026 20:37:27 -0500 Subject: [PATCH 03/23] Repair hosted leak analysis execution Build Linux targets without entering the package deployment path, and ignore Dr. Memory's incomplete Windows bootstrap report while retaining it in the raw artifact. Files changed: - .github/scripts/run-drmemory.ps1 - .github/workflows/memory-leak-analysis.yml Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- .github/scripts/run-drmemory.ps1 | 7 +++++-- .github/workflows/memory-leak-analysis.yml | 8 +++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/.github/scripts/run-drmemory.ps1 b/.github/scripts/run-drmemory.ps1 index 324baf3ad..5e00545e7 100644 --- a/.github/scripts/run-drmemory.ps1 +++ b/.github/scripts/run-drmemory.ps1 @@ -32,7 +32,7 @@ function Get-LeakCount { ) $escapedCategory = [regex]::Escape($Category) - $pattern = "(?m)^\s*~~Dr\.M~~\s+([\d,]+) unique,\s+([\d,]+) total,\s+([\d,]+) byte\(s\) of $escapedCategory\r?$" + $pattern = "(?m)^\s*(?:~~Dr\.M~~\s+)?([\d,]+) unique,\s+([\d,]+) total,\s+([\d,]+) byte\(s\) of $escapedCategory\r?$" $match = [regex]::Match($Results, $pattern) if (-not $match.Success) { throw "Dr. Memory results do not contain the '$Category' summary." @@ -65,8 +65,11 @@ if ($targetExitCode -ne 0) { } $resultFiles = @(Get-ChildItem -LiteralPath $scenarioDirectory -Filter results.txt -File -Recurse) +$resultFiles = @($resultFiles | Where-Object { + Select-String -LiteralPath $_.FullName -Pattern '^ERRORS FOUND:\r?$' -Quiet +}) if ($resultFiles.Count -ne 1) { - throw "Expected one Dr. Memory results.txt for $Scenario, found $($resultFiles.Count)." + throw "Expected one completed Dr. Memory results.txt for $Scenario, found $($resultFiles.Count)." } $results = Get-Content -LiteralPath $resultFiles[0].FullName -Raw diff --git a/.github/workflows/memory-leak-analysis.yml b/.github/workflows/memory-leak-analysis.yml index 0ec2ba3ab..50a82d38c 100644 --- a/.github/workflows/memory-leak-analysis.yml +++ b/.github/workflows/memory-leak-analysis.yml @@ -115,9 +115,11 @@ jobs: run: git submodule update --init --depth=1 third_party/googletest - name: Build leak-analysis targets - env: - CMAKE_OPTS: -DMATSDK_BUILD_UNIT_TESTS=ON -DMATSDK_BUILD_FUNC_TESTS=ON - run: ./build.sh debug + run: | + cmake --preset matsdk-debug \ + -DMATSDK_BUILD_UNIT_TESTS=ON \ + -DMATSDK_BUILD_FUNC_TESTS=ON + cmake --build --preset matsdk-debug --parallel 2 - name: Build basic sample run: | From fe74bd73eaab92594076c080af7c7fa4b3e0a552 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Thu, 17 Sep 2026 21:10:09 -0500 Subject: [PATCH 04/23] Stabilize hosted leak scenarios Install the Linux curl development dependency, avoid the unrelated installed-package target regression when compiling the sample, and exclude the one functional assertion whose exact asynchronous drop count changes under instrumentation. Files changed: - .github/workflows/memory-leak-analysis.yml Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- .github/workflows/memory-leak-analysis.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/memory-leak-analysis.yml b/.github/workflows/memory-leak-analysis.yml index 50a82d38c..541d79d3c 100644 --- a/.github/workflows/memory-leak-analysis.yml +++ b/.github/workflows/memory-leak-analysis.yml @@ -82,6 +82,7 @@ jobs: -LogDirectory drmemory-results -Scenario functional-tests -TargetPath Solutions/out/Debug/x64/FuncTests/FuncTests.exe + -TargetArguments "--gtest_filter=-BasicFuncTests.killSwitchWorks" - name: Analyze basic sample shell: pwsh @@ -114,6 +115,11 @@ jobs: - name: Initialize googletest run: git submodule update --init --depth=1 third_party/googletest + - name: Install build dependencies + run: | + sudo apt-get update + sudo apt-get install -y libcurl4-openssl-dev + - name: Build leak-analysis targets run: | cmake --preset matsdk-debug \ @@ -126,6 +132,7 @@ jobs: cmake --install out --prefix "$PWD/out/install" cmake -S examples/cpp/SampleCppMini -B out/sample-cpp-mini \ -DCMAKE_BUILD_TYPE=Debug \ + -DCMAKE_DISABLE_FIND_PACKAGE_MSTelemetry=TRUE \ -DMATSDK_INSTALL_DIR="$PWD/out/install" cmake --build out/sample-cpp-mini --parallel 2 @@ -156,7 +163,7 @@ jobs: -LogDirectory drmemory-results -Scenario functional-tests -TargetPath out/tests/functests/FuncTests - -TargetArguments "--gtest_filter=-APITest.C_API_Test" + -TargetArguments "--gtest_filter=-APITest.C_API_Test:BasicFuncTests.killSwitchWorks" - name: Analyze basic sample shell: pwsh From ba167fd018ac53c624127739c98a81d2f9889273 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Thu, 17 Sep 2026 21:44:12 -0500 Subject: [PATCH 05/23] Make the sample portable under leak analysis Recognize Dr. Memory's clean-report marker and disambiguate SampleCppMini's signed 64-bit EventProperty construction so the same sample compiles under GCC and MSVC. Files changed: - .github/scripts/run-drmemory.ps1 - examples/cpp/SampleCppMini/main.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- .github/scripts/run-drmemory.ps1 | 2 +- examples/cpp/SampleCppMini/main.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/scripts/run-drmemory.ps1 b/.github/scripts/run-drmemory.ps1 index 5e00545e7..a56c7eb98 100644 --- a/.github/scripts/run-drmemory.ps1 +++ b/.github/scripts/run-drmemory.ps1 @@ -66,7 +66,7 @@ if ($targetExitCode -ne 0) { $resultFiles = @(Get-ChildItem -LiteralPath $scenarioDirectory -Filter results.txt -File -Recurse) $resultFiles = @($resultFiles | Where-Object { - Select-String -LiteralPath $_.FullName -Pattern '^ERRORS FOUND:\r?$' -Quiet + Select-String -LiteralPath $_.FullName -Pattern '^(?:NO )?ERRORS FOUND:\r?$' -Quiet }) if ($resultFiles.Count -ne 1) { throw "Expected one completed Dr. Memory results.txt for $Scenario, found $($resultFiles.Count)." diff --git a/examples/cpp/SampleCppMini/main.cpp b/examples/cpp/SampleCppMini/main.cpp index 9a94146ce..c446777a3 100644 --- a/examples/cpp/SampleCppMini/main.cpp +++ b/examples/cpp/SampleCppMini/main.cpp @@ -77,7 +77,7 @@ void test_cpp_api(const char * token, int ticketType, const char *ticket) // Various typed key-values { "strKey1", "hello1" }, { "strKey2", "hello2" }, - { "int64Key", 1LL }, + { "int64Key", static_cast(1) }, { "dblKey", 3.14 }, { "boolKey", false }, { "guidKey0", GUID_t("00000000-0000-0000-0000-000000000000") }, From c1888e5ae86d3560bf3b870ea4b68eccf48e2148 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 18 Sep 2026 03:09:09 -0500 Subject: [PATCH 06/23] Stop loading the leaking Windows Network List Manager Use Windows.Networking.Connectivity for both cost queries and change notifications so network detection preserves behavior without instantiating PublicNetworkListManager or loading netprofm.dll. Fail periodic leak analysis if netprofm returns. Files changed: - lib/pal/desktop/NetworkDetector.cpp - lib/pal/desktop/NetworkDetector.hpp - .github/workflows/memory-leak-analysis.yml Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- .github/workflows/memory-leak-analysis.yml | 10 ++ lib/pal/desktop/NetworkDetector.cpp | 145 ++++++++------------- lib/pal/desktop/NetworkDetector.hpp | 2 + 3 files changed, 69 insertions(+), 88 deletions(-) diff --git a/.github/workflows/memory-leak-analysis.yml b/.github/workflows/memory-leak-analysis.yml index 541d79d3c..a7e82c300 100644 --- a/.github/workflows/memory-leak-analysis.yml +++ b/.github/workflows/memory-leak-analysis.yml @@ -93,6 +93,16 @@ jobs: -Scenario sample-cpp-mini -TargetPath Solutions/out/Debug/x64/SampleCppMini/SampleCppMini.exe + - name: Verify Network List Manager is not loaded + shell: pwsh + run: | + $matches = Get-ChildItem drmemory-results -Filter *.txt -File -Recurse | + Select-String -Pattern 'netprofm\.dll' + if ($matches) { + $matches | ForEach-Object { Write-Error "$($_.Path):$($_.LineNumber): $($_.Line)" } + throw "Network detection loaded netprofm.dll." + } + - name: Upload Windows reports if: always() uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 diff --git a/lib/pal/desktop/NetworkDetector.cpp b/lib/pal/desktop/NetworkDetector.cpp index f1a90e5b8..474f4f115 100644 --- a/lib/pal/desktop/NetworkDetector.cpp +++ b/lib/pal/desktop/NetworkDetector.cpp @@ -141,46 +141,51 @@ namespace MAT_NS_BEGIN NetworkCost result = NetworkCost_Unknown; LOG_TRACE("get network cost...\n"); - if (pNlm == NULL) { - LOG_WARN("INetworkCostManager is unavailable!"); + if (networkInfoStats == nullptr) { + LOG_WARN("Windows network information is unavailable!"); return result; } - HRESULT hr; + ComPtr connectionProfile; + HRESULT hr = networkInfoStats->GetInternetConnectionProfile(&connectionProfile); + if (FAILED(hr) || connectionProfile == nullptr) { + return result; + } - DWORD dwCost = NLM_CONNECTION_COST_UNKNOWN; - INetworkCostManager* pNetworkCostManager = NULL; + ComPtr connectionCost; + hr = connectionProfile->GetConnectionCost(&connectionCost); + if (FAILED(hr) || connectionCost == nullptr) { + return result; + } - hr = pNlm->QueryInterface(IID_INetworkCostManager2, (void**)&pNetworkCostManager); - if (hr != S_OK) { + boolean roaming = false; + boolean overDataLimit = false; + NetworkCostType costType = NetworkCostType_Unknown; + if (FAILED(connectionCost->get_Roaming(&roaming)) || + FAILED(connectionCost->get_OverDataLimit(&overDataLimit)) || + FAILED(connectionCost->get_NetworkCostType(&costType))) { return result; } - hr = pNetworkCostManager->GetCost(&dwCost, NULL); - if (hr == S_OK) { - switch (dwCost) { - case NLM_CONNECTION_COST_UNRESTRICTED: // The connection is unlimited and is considered to be unrestricted of usage charges and capacity constraints. - result = NetworkCost_Unmetered; - break; - case NLM_CONNECTION_COST_FIXED: // The use of this connection is unrestricted up to a specific data transfer limit. - case NLM_CONNECTION_COST_VARIABLE: // This connection is regulated on a per byte basis. - result = NetworkCost_Metered; - break; - case NLM_CONNECTION_COST_OVERDATALIMIT: // The connection is currently in an OverDataLimit state as it has exceeded the carrier specified data transfer limit. - case NLM_CONNECTION_COST_CONGESTED: // The network is experiencing high traffic load and is congested. - case NLM_CONNECTION_COST_ROAMING: // The connection is roaming outside the network and affiliates of the home provider. - case NLM_CONNECTION_COST_APPROACHINGDATALIMIT: // The connection is approaching the data limit specified by the carrier. - result = NetworkCost_Roaming; - break; - case NLM_CONNECTION_COST_UNKNOWN: - default: - result = NetworkCost_Unknown; // The cost is unknown. - break; - } + if (roaming || overDataLimit) { + return NetworkCost_Roaming; + } + + switch (costType) { + case NetworkCostType_Unrestricted: + result = NetworkCost_Unmetered; + break; + case NetworkCostType_Fixed: + case NetworkCostType_Variable: + result = NetworkCost_Metered; + break; + case NetworkCostType_Unknown: + default: + break; } return result; -} + } /// /// Get adapter id for IConnectionProfile @@ -359,53 +364,25 @@ namespace MAT_NS_BEGIN bool NetworkDetector::RegisterAndListen() noexcept { - // ??? - HRESULT hr = pNlm->QueryInterface(IID_IUnknown, (void**)&pSink); - if (FAILED(hr)) - { - LOG_ERROR("cannot query IID_IUnknown!!!"); + networkStatusChangedHandler = Callback( + [this](IInspectable*) -> HRESULT { + GetCurrentNetworkCost(); + return S_OK; + }); + if (networkStatusChangedHandler == nullptr) { + LOG_ERROR("Unable to create network status handler."); return false; } - pSink = (INetworkEvents*)this; - - hr = pNlm->QueryInterface(IID_IConnectionPointContainer, (void**)&pCpc); - if (FAILED(hr)) - { - LOG_ERROR("Unable to QueryInterface IID_IConnectionPointContainer!"); + HRESULT hr = networkInfoStats->add_NetworkStatusChanged( + networkStatusChangedHandler.Get(), + &networkStatusChangedToken); + if (FAILED(hr)) { + LOG_ERROR("Unable to subscribe to network status changes."); + networkStatusChangedHandler.Reset(); return false; } - hr = pCpc->FindConnectionPoint(IID_INetworkConnectionEvents, &m_pc1); - if (SUCCEEDED(hr)) - { - hr = m_pc1->Advise( - pSink.Get(), - &m_dwCookie_INetworkConnectionEvents); - LOG_INFO("listening to INetworkConnectionEvents... %s", - (SUCCEEDED(hr)) ? "OK" : "FAILED"); - } - - hr = pCpc->FindConnectionPoint(IID_INetworkEvents, &m_pc2); - if (SUCCEEDED(hr)) - { - hr = m_pc2->Advise( - pSink.Get(), - &m_dwCookie_INetworkEvents); - LOG_INFO("listening to INetworkEvents... %s", - (SUCCEEDED(hr)) ? "OK" : "FAILED"); - } - - hr = pCpc->FindConnectionPoint(IID_INetworkListManagerEvents, &m_pc3); - if (SUCCEEDED(hr)) - { - hr = m_pc3->Advise( - pSink.Get(), - &m_dwCookie_INetworkListManagerEvents); - LOG_INFO("listening to INetworkListManagerEvents... %s", - (SUCCEEDED(hr)) ? "OK" : "FAILED"); - } - MSG msg; PostThreadMessage(m_listener_tid, NETDETECTOR_START, 0, 0); cv.notify_all(); @@ -431,6 +408,13 @@ namespace MAT_NS_BEGIN /// void NetworkDetector::Reset() { + if (networkStatusChangedToken.value != 0 && networkInfoStats != nullptr) + { + networkInfoStats->remove_NetworkStatusChanged(networkStatusChangedToken); + networkStatusChangedToken.value = 0; + } + networkStatusChangedHandler.Reset(); + if (m_pc1 != nullptr) { m_pc1->Unadvise(m_dwCookie_INetworkConnectionEvents); @@ -498,24 +482,9 @@ namespace MAT_NS_BEGIN isCoInitialized = true; if (GetNetworkInfoStats()) { - LOG_INFO("create network list manager..."); - hr = CoCreateInstance( - CLSID_NetworkListManager, - nullptr, - CLSCTX_ALL, - IID_INetworkListManager, - (void**)&pNlm); - if (FAILED(hr)) - { - LOG_ERROR("Unable to CoCreateInstance for CLSID_NetworkListManager!"); - } - else - { - GetCurrentNetworkCost(); - LOG_TRACE("start listening to events..."); - RegisterAndListen(); // we block here to process COM events - } - // Once we are done OR cannot init NLM, we must perform the clean-up + GetCurrentNetworkCost(); + LOG_TRACE("start listening to events..."); + RegisterAndListen(); Reset(); } } diff --git a/lib/pal/desktop/NetworkDetector.hpp b/lib/pal/desktop/NetworkDetector.hpp index 1404334b4..c3eae33df 100644 --- a/lib/pal/desktop/NetworkDetector.hpp +++ b/lib/pal/desktop/NetworkDetector.hpp @@ -94,6 +94,8 @@ namespace MAT_NS_BEGIN /// Current network info stats /// ComPtr networkInfoStats; + ComPtr networkStatusChangedHandler; + EventRegistrationToken networkStatusChangedToken{}; /// From f69315437e97fbcf5292060370dc121b8138e80f Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 18 Sep 2026 03:16:12 -0500 Subject: [PATCH 07/23] Release memory allocated by unit tests Use scoped objects for temporary event and buffer allocations, and destroy the log-session provider during fixture teardown so leak reports represent SDK behavior rather than test fixture ownership. Files changed: tests/unittests/AnnexKTests.cpp tests/unittests/LogSessionDataDBTests.cpp tests/unittests/TransmissionPolicyManagerTests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- tests/unittests/AnnexKTests.cpp | 30 ++++++++-------- tests/unittests/LogSessionDataDBTests.cpp | 3 +- .../TransmissionPolicyManagerTests.cpp | 34 +++++++++---------- 3 files changed, 34 insertions(+), 33 deletions(-) diff --git a/tests/unittests/AnnexKTests.cpp b/tests/unittests/AnnexKTests.cpp index fa74e23f5..3b2596694 100644 --- a/tests/unittests/AnnexKTests.cpp +++ b/tests/unittests/AnnexKTests.cpp @@ -8,25 +8,25 @@ TEST(AnnexKTests, memcpy_s) { volatile size_t dest_size =10; volatile size_t src_size = 5; - void *dest = malloc(sizeof(char) * dest_size); - void *src = malloc(sizeof(char) * src_size); - EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(src, 5, "TEST", 5), 0); + std::unique_ptr dest(malloc(sizeof(char) * dest_size), &free); + std::unique_ptr src(malloc(sizeof(char) * src_size), &free); + EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(src.get(), 5, "TEST", 5), 0); rsize_t dest_len = dest_size; rsize_t src_len = src_size-1; // success tests - EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(dest, dest_len, src, 0), 0); - EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s( dest, dest_len, src, src_len + 1), 0); - EXPECT_EQ(strlen((char *)dest), strlen("TEST")); - EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(dest, dest_len + 2, src, src_len + 1), 0); - EXPECT_EQ(strlen((char *)dest), strlen("TEST")); + EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(dest.get(), dest_len, src.get(), 0), 0); + EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(dest.get(), dest_len, src.get(), src_len + 1), 0); + EXPECT_EQ(strlen(static_cast(dest.get())), strlen("TEST")); + EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(dest.get(), dest_len + 2, src.get(), src_len + 1), 0); + EXPECT_EQ(strlen(static_cast(dest.get())), strlen("TEST")); // error tests - EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(dest, 3, src, src_len), EINVAL); - EXPECT_EQ(((char *)dest)[0], '\0'); - EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(NULL, 3, src, src_len), EINVAL); - EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s( dest, dest_len, NULL, src_len), EINVAL); - EXPECT_EQ(((char *)dest)[0], '\0'); - EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s( dest, dest_len, src, dest_len + 1 ), EINVAL); - EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s( dest, dest_len, (void *)((char *)dest + 1), src_len + 1 ), EINVAL); + EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(dest.get(), 3, src.get(), src_len), EINVAL); + EXPECT_EQ(static_cast(dest.get())[0], '\0'); + EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(NULL, 3, src.get(), src_len), EINVAL); + EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(dest.get(), dest_len, NULL, src_len), EINVAL); + EXPECT_EQ(static_cast(dest.get())[0], '\0'); + EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(dest.get(), dest_len, src.get(), dest_len + 1), EINVAL); + EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(dest.get(), dest_len, static_cast(dest.get()) + 1, src_len + 1), EINVAL); } diff --git a/tests/unittests/LogSessionDataDBTests.cpp b/tests/unittests/LogSessionDataDBTests.cpp index 4788c5302..594a20c13 100644 --- a/tests/unittests/LogSessionDataDBTests.cpp +++ b/tests/unittests/LogSessionDataDBTests.cpp @@ -72,6 +72,8 @@ class LogSessionDataDBTests : public ::testing::Test virtual void TearDown() override { + delete logSessionDataProvider; + logSessionDataProvider = nullptr; std::remove(name.str().c_str()); offlineStorage->Shutdown(); offlineStorage.reset(); @@ -97,4 +99,3 @@ TEST_F(LogSessionDataDBTests, subTest) { ASSERT_EQ(1, 1); #endif } - diff --git a/tests/unittests/TransmissionPolicyManagerTests.cpp b/tests/unittests/TransmissionPolicyManagerTests.cpp index c2ce2c3ae..d0b052a04 100644 --- a/tests/unittests/TransmissionPolicyManagerTests.cpp +++ b/tests/unittests/TransmissionPolicyManagerTests.cpp @@ -153,8 +153,8 @@ TEST_F(TransmissionPolicyManagerTests, IncomingEventDoesNothingWhenPaused) { tpm.paused(true); - auto event = new IncomingEventContext(); - tpm.eventArrived(event); + IncomingEventContext event; + tpm.eventArrived(&event); } TEST_F(TransmissionPolicyManagerTests, IncomingEventSchedulesUpload) @@ -174,13 +174,13 @@ TEST_F(TransmissionPolicyManagerTests, IncomingEventSchedulesUpload) EXPECT_TRUE(TransmitProfiles::load(customProfile)); EXPECT_TRUE(TransmitProfiles::setProfile("Fred")); - auto event = new IncomingEventContext(); - event->record.latency = EventLatency_Normal; + IncomingEventContext event; + event.record.latency = EventLatency_Normal; EXPECT_CALL(tpm, scheduleUpload(std::chrono::milliseconds { 1000 }, EventLatency_Normal, true)) .WillOnce(Return()); - tpm.eventArrived(event); + tpm.eventArrived(&event); } TEST_F(TransmissionPolicyManagerTests, ProfileAffectsSchedule) @@ -200,10 +200,10 @@ TEST_F(TransmissionPolicyManagerTests, ProfileAffectsSchedule) EXPECT_TRUE(TransmitProfiles::load(customProfile)); EXPECT_TRUE(TransmitProfiles::setProfile("Fred")); - auto event = new IncomingEventContext(); - event->record.latency = EventLatency_Normal; + IncomingEventContext event; + event.record.latency = EventLatency_Normal; EXPECT_CALL(tpm, scheduleUpload(_, _, _)).Times(0); - tpm.eventArrived(event); + tpm.eventArrived(&event); TransmitProfiles::reset(); } @@ -224,10 +224,10 @@ TEST_F(TransmissionPolicyManagerTests, NoUploadForNegative) EXPECT_TRUE(TransmitProfiles::load(customProfile)); EXPECT_TRUE(TransmitProfiles::setProfile("Fred")); - auto event = new IncomingEventContext(); - event->record.latency = EventLatency_Normal; + IncomingEventContext event; + event.record.latency = EventLatency_Normal; EXPECT_CALL(tpm, scheduleUpload(_, _, _)).Times(0); - tpm.eventArrived(event); + tpm.eventArrived(&event); EXPECT_CALL(tpm, uploadAsync(_)).Times(0); tpm.scheduleUploadParent(std::chrono::milliseconds{-1000}, EventLatency_RealTime, true); TransmitProfiles::reset(); @@ -237,12 +237,12 @@ TEST_F(TransmissionPolicyManagerTests, ImmediateIncomingEventStartsUploadImmedia { tpm.paused(false); - auto event = new IncomingEventContext(); - event->record.latency = EventLatency_Max; + IncomingEventContext event; + event.record.latency = EventLatency_Max; EventsUploadContextPtr upload; EXPECT_CALL(*this, resultInitiateUpload(_)) .WillOnce(SaveArg<0>(&upload)); - tpm.eventArrived(event); + tpm.eventArrived(&event); ASSERT_THAT(upload, NotNull()); EXPECT_THAT(upload->requestedMinLatency, EventLatency_Max); @@ -491,11 +491,11 @@ TEST_F(TransmissionPolicyManagerTests, FredProfile) EXPECT_TRUE(TransmitProfiles::setProfile("Fred_Profile")); tpm.paused(false); - auto event = new IncomingEventContext(); - event->record.latency = EventLatency_Normal; + IncomingEventContext event; + event.record.latency = EventLatency_Normal; EXPECT_CALL(tpm, scheduleUpload(_, _, _)) .Times(0); - tpm.eventArrived(event); + tpm.eventArrived(&event); } TEST_F(TransmissionPolicyManagerTests, Constructor_IsPaused_True) From 77d6581f47198922531efd3ef15ce59f1c3a7557 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 18 Sep 2026 03:27:53 -0500 Subject: [PATCH 08/23] Verify WinRT network detection lifecycle Exercise the real Windows detector so CI proves that WinRT status registration starts, network cost remains valid, shutdown completes, and netprofm.dll is not loaded. Files changed: tests/unittests/NetworkDetectorTests.cpp tests/unittests/CMakeLists.txt tests/unittests/UnitTests.vcxproj Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- tests/unittests/CMakeLists.txt | 4 ++++ tests/unittests/NetworkDetectorTests.cpp | 30 ++++++++++++++++++++++++ tests/unittests/UnitTests.vcxproj | 1 + 3 files changed, 35 insertions(+) create mode 100644 tests/unittests/NetworkDetectorTests.cpp diff --git a/tests/unittests/CMakeLists.txt b/tests/unittests/CMakeLists.txt index e973ccf4c..e27912041 100644 --- a/tests/unittests/CMakeLists.txt +++ b/tests/unittests/CMakeLists.txt @@ -75,6 +75,10 @@ if (APPLE) endif() endif() +if (WIN32) + list(APPEND SRCS NetworkDetectorTests.cpp) +endif() + if (EXISTS "${PROJECT_SOURCE_DIR}/lib/modules/exp/tests") list(APPEND SRCS "${PROJECT_SOURCE_DIR}/lib/modules/exp/tests/unittests/ECSConfigCacheTests.cpp" diff --git a/tests/unittests/NetworkDetectorTests.cpp b/tests/unittests/NetworkDetectorTests.cpp new file mode 100644 index 000000000..84d08092d --- /dev/null +++ b/tests/unittests/NetworkDetectorTests.cpp @@ -0,0 +1,30 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. + +#include "common/Common.hpp" + +#ifdef _WIN32 +#include "pal/desktop/NetworkDetector.hpp" + +using namespace MAT; +using namespace testing; + +TEST(NetworkDetectorTests, StartsReadsCostAndStopsWithoutNetworkListManager) +{ + ASSERT_EQ(GetModuleHandleW(L"netprofm.dll"), nullptr); + + MATW::NetworkDetector detector; + ASSERT_TRUE(detector.Start()); + EXPECT_TRUE(detector.isUp()); + + const auto cost = detector.GetCurrentNetworkCost(); + EXPECT_THAT(cost, AnyOf( + Eq(NetworkCost_Unknown), + Eq(NetworkCost_Unmetered), + Eq(NetworkCost_Metered), + Eq(NetworkCost_Roaming))); + + detector.Stop(); + EXPECT_FALSE(detector.isUp()); + EXPECT_EQ(GetModuleHandleW(L"netprofm.dll"), nullptr); +} +#endif diff --git a/tests/unittests/UnitTests.vcxproj b/tests/unittests/UnitTests.vcxproj index faf465e97..d2331745d 100644 --- a/tests/unittests/UnitTests.vcxproj +++ b/tests/unittests/UnitTests.vcxproj @@ -442,6 +442,7 @@ + From e948a2c7b9cbcc28907cff68ab084cf9e6ad94d6 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 18 Sep 2026 03:44:25 -0500 Subject: [PATCH 09/23] Remove obsolete Network List Manager state Delete the unused NLM interfaces, connection points, callbacks, maps, compatibility branch, and manual reference counting now that network detection is entirely WinRT-based. This reduces object and binary overhead while keeping ownership with unique_ptr. Files changed: docs/building-custom-SKU.md lib/pal/desktop/NetworkDetector.cpp lib/pal/desktop/NetworkDetector.hpp lib/pal/desktop/WindowsDesktopNetworkInformationImpl.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- docs/building-custom-SKU.md | 2 +- lib/pal/desktop/NetworkDetector.cpp | 430 +----------------- lib/pal/desktop/NetworkDetector.hpp | 156 +------ .../WindowsDesktopNetworkInformationImpl.cpp | 3 - 4 files changed, 20 insertions(+), 571 deletions(-) diff --git a/docs/building-custom-SKU.md b/docs/building-custom-SKU.md index a68d6a681..472644ef6 100644 --- a/docs/building-custom-SKU.md +++ b/docs/building-custom-SKU.md @@ -28,7 +28,7 @@ Build recipe must contain the following preprocessor definitions: | HAVE_MAT_WIN_LOG | off | Will log statements to disk on windows if trace enabled and HAVE_MAT_LOGGING defined | | HAVE_MAT_EVT_TRACEID | off | Enable event tracking by adding trace-id to http request header on Windows. This is for debugging purpose, and not recommended to be enabled in production. The collector doesn't parse/read this header. As of now, this is meant to be used through the capi, where the http-send handler should remove this header from the event data before sending it to collector. | | HAVE_MAT_STORAGE | on | Enable SQLite persistent offline storage | -| HAVE_MAT_NETDETECT | on | _Win32 Desktop only_: Use NLM COM object for network cost detection on Windows 8+ | +| HAVE_MAT_NETDETECT | on | _Win32 Desktop only_: Use Windows Runtime APIs for network cost detection on Windows 8+ | | HAVE_MAT_SHORT_NS | off | Use short "MAT::" namespace instead of "Microsoft::Applications::Events::" to reduce the .DLL size | | HAVE_CS4 | off | Build with Common Schema 4.0 support. Current default is `off`, i.e. building with Common Schema 3.0 support | | HAVE_CS4_FULL | off | Enable additional Common Schema 4.0 protocol features needed by server / services SDK | diff --git a/lib/pal/desktop/NetworkDetector.cpp b/lib/pal/desktop/NetworkDetector.cpp index 474f4f115..caa7e55a4 100644 --- a/lib/pal/desktop/NetworkDetector.cpp +++ b/lib/pal/desktop/NetworkDetector.cpp @@ -8,91 +8,32 @@ #pragma comment(lib, "runtimeobject.lib") -// This macro is required for DEFINE_GUID below to declare a local instance of IID_INetworkCostManager GUID -#define INITGUID - #include "NetworkDetector.hpp" -#include #include "ILogManager.hpp" #include "DebugEvents.hpp" -#include "utils/Utils.hpp" #include "pal/PAL.hpp" -// Define a GUID that is only available in Windows 8.x+ SDK . We are using Windows 7.1A SDK for Win32 SDK build, -// so we cannot easily add an extra dependency on Windows 8 or later functionality project-wide. It'd be error-prone, -// because when we have all Windows 8+ features - we might fall into temptation of using that features that would -// break Windows 7.1 compatibility. We cannot afford breaking Windows 7.1 compatibility at this time. -DEFINE_GUID(IID_INetworkCostManager2, 0xdcb00008, 0x570f, 0x4a9b, 0x8d, 0x69, 0x19, 0x9f, 0xdb, 0xa5, 0x72, 0x3b); - -#define NETDETECTOR_START WM_USER+1 -#define NETDETECTOR_STOP WM_USER+2 - -#define NETDETECTOR_COM_SETTLE_MS 1000 +#define NETDETECTOR_STOP WM_USER+1 +#define NETDETECTOR_START_TIMEOUT_MS 1000 namespace MAT_NS_BEGIN { namespace Windows { - // Malwarebytes have been detected - static bool mbDetected = false; - - /// - /// Convert HString to std::string - /// - /// - /// - std::string to_string(HString *name) - { - UINT32 length; - PCWSTR rawString = name->GetRawBuffer(&length); - std::wstring wide(rawString); - return to_utf8_string(wide); - } - - /// - /// Convert GUID to std::string - /// - /// - /// - std::string to_string(GUID guid) { - std::string result; - char buff[40] = { 0 }; // Maximum hyphenated GUID length with braces is 38 + null terminator - sprintf_s(buff, sizeof(buff), - "{%08lX-%04hX-%04hX-%02hhX%02hhX-%02hhX%02hhX%02hhX%02hhX%02hhX%02hhX}", - guid.Data1, guid.Data2, guid.Data3, - guid.Data4[0], guid.Data4[1], guid.Data4[2], guid.Data4[3], - guid.Data4[4], guid.Data4[5], guid.Data4[6], guid.Data4[7]); - result = buff; - return result; - } - NetworkCost const& NetworkDetector::GetNetworkCost() { - return (NetworkCost const &)m_currentNetworkCost; - } - - NetworkType NetworkDetector::GetNetworkType() - { - return m_currentNetworkType.load(); + return m_currentNetworkCost; } /// /// Get current realtime network cost synchronously. - /// This function can be called on any Windows release and it provides a SEH handler. + /// This function provides an SEH handler for Windows Runtime failures. /// /// #pragma warning(push) #pragma warning(disable: 6320) int NetworkDetector::GetCurrentNetworkCost() { -#if 0 - // We don't know the cost of something that is not there - if (m_connectivity == NLM_CONNECTIVITY_DISCONNECTED) - { - TRACE("Disconnected!"); - m_currentNetworkCost = NetworkCost_Unknown; - } -#endif m_currentNetworkCost = NetworkCost_Unknown; __try { m_currentNetworkCost = _GetCurrentNetworkCost(); @@ -116,22 +57,13 @@ namespace MAT_NS_BEGIN DebugEvent evt; evt.type = DebugEventType::EVT_NET_CHANGED; evt.param1 = m_currentNetworkCost; - evt.param2 = mbDetected; + evt.param2 = false; ILogManager::DispatchEventBroadcast(evt); return m_currentNetworkCost; } #pragma warning(pop) - /// - /// Get current network connectivity state - /// - /// Value of enum NLM_CONNECTIVITY - int NetworkDetector::GetConnectivity() - { - return m_connectivity; - } - /// /// Internal implementation /// @@ -187,166 +119,6 @@ namespace MAT_NS_BEGIN return result; } - /// - /// Get adapter id for IConnectionProfile - /// - /// - /// - std::string NetworkDetector::GetAdapterId(IConnectionProfile *profile) - { - if (!profile) - { - LOG_ERROR("Invalid profile pointer!"); - return ""; // Invalid interface ptr - } - -#if 0 /* FIXME: do we return none if connectivity level is none? */ - NetworkConnectivityLevel connectivityLevel; - HRESULT hr = profile->GetNetworkConnectivityLevel(&connectivityLevel); - if (connectivityLevel != NetworkConnectivityLevel_None) - { - - } -#endif - - ComPtr adapter; - HRESULT hr = profile->get_NetworkAdapter(&adapter); - if (hr == E_INVALIDARG) - { - // No interfaces - device is in airplane mode - LOG_TRACE("No network interfaces - device is in airplane mode"); - return ""; - } - - GUID id; - hr = adapter->get_NetworkAdapterId(&id); - if (!SUCCEEDED(hr)) - { - // Unable to obtain Network Adapter GUID - LOG_TRACE("Unable to obtain interface GUID"); - return ""; - } - - return to_string(id); - } - - /// - /// COM thread interfaces supported by this class - /// - /// - /// - /// - HRESULT NetworkDetector::QueryInterface(REFIID riid, void ** ppv) noexcept - { - if (!ppv) - { - return E_POINTER; - } - - *ppv = nullptr; - HRESULT hr = E_NOINTERFACE; - - if (IID_INetworkEvents == riid) - { - *ppv = static_cast(this); - hr = S_OK; - } - else if (IID_INetworkConnectionEvents == riid) - { - *ppv = static_cast(this); - hr = S_OK; - } - else if (IID_INetworkListManagerEvents == riid) - { - *ppv = static_cast(this); - hr = S_OK; - } - else if (IID_IUnknown == riid) - { - *ppv = static_cast(static_cast(this)); - hr = S_OK; - } - - if (SUCCEEDED(hr)) - { - AddRef(); - } - - return hr; - } - - ULONG NetworkDetector::AddRef(void) noexcept - { - return InterlockedIncrement((LONG *)&m_lRef); - } - - ULONG NetworkDetector::Release(void) noexcept - { - ULONG ulNewRef = (ULONG)InterlockedDecrement((LONG *)&m_lRef); - if (ulNewRef == 0) - { - // NetworkDetector is destroyed from FlushAndTeardown. - // If customer forgets to call it, then it is destroyed from atexit(...) - LOG_TRACE("NetworkDetector last instance released (this=%p)", this); - } - return ulNewRef; - } - - HRESULT NetworkDetector::ConnectivityChanged(NLM_CONNECTIVITY newConnectivity) - { - LOG_TRACE("Connectivity changed: %d", newConnectivity); - m_connectivity = newConnectivity; - GetCurrentNetworkCost(); - return RPC_S_OK; - } - - HRESULT NetworkDetector::NetworkAdded(GUID networkId) - { - LOG_TRACE("NetworkAdded: %s", to_string(networkId).c_str()); - m_networks.push_back(to_string(networkId)); - return RPC_S_OK; - } - - HRESULT NetworkDetector::NetworkDeleted(GUID networkId) - { - LOG_TRACE("NetworkDeleted: %s", to_string(networkId).c_str()); - auto &v = m_networks; - const std::string &item = to_string(networkId); - v.erase(std::remove(v.begin(), v.end(), item), v.end()); - return RPC_S_OK; - } - - HRESULT NetworkDetector::NetworkConnectivityChanged(GUID networkId, NLM_CONNECTIVITY newConnectivity) - { - LOG_TRACE("NetworkConnectivityChanged: %s, %d", to_string(networkId).c_str(), newConnectivity); - m_networks_connectivity[to_string(networkId)] = newConnectivity; - return RPC_S_OK; - } - - HRESULT NetworkDetector::NetworkPropertyChanged(GUID networkId, NLM_NETWORK_PROPERTY_CHANGE flags) - { - UNREFERENCED_PARAMETER(networkId); - UNREFERENCED_PARAMETER(flags); - LOG_TRACE("NetworkPropertyChanged: %s, %d", to_string(networkId).c_str(), flags); - GetCurrentNetworkCost(); - return RPC_S_OK; - } - - HRESULT NetworkDetector::NetworkConnectionConnectivityChanged(GUID connectionId, NLM_CONNECTIVITY newConnectivity) - { - LOG_TRACE("NetworkConnectionConnectivityChanged: %s, %d", to_string(connectionId).c_str(), newConnectivity); - m_connections_connectivity[to_string(connectionId)] = newConnectivity; - return RPC_S_OK; - } - - HRESULT NetworkDetector::NetworkConnectionPropertyChanged(GUID connectionId, NLM_CONNECTION_PROPERTY_CHANGE flags) - { - UNREFERENCED_PARAMETER(connectionId); - UNREFERENCED_PARAMETER(flags); - LOG_TRACE("NetworkConnectionPropertyChanged: %s, %d", to_string(connectionId).c_str(), flags); - return RPC_S_OK; - } - /// /// Get activation factory and look-up network info statistics /// @@ -384,7 +156,7 @@ namespace MAT_NS_BEGIN } MSG msg; - PostThreadMessage(m_listener_tid, NETDETECTOR_START, 0, 0); + PeekMessage(&msg, nullptr, WM_USER, WM_USER, PM_NOREMOVE); cv.notify_all(); while (GetMessage(&msg, NULL, 0, 0) > 0) @@ -414,61 +186,17 @@ namespace MAT_NS_BEGIN networkStatusChangedToken.value = 0; } networkStatusChangedHandler.Reset(); - - if (m_pc1 != nullptr) - { - m_pc1->Unadvise(m_dwCookie_INetworkConnectionEvents); - m_pc1 = nullptr; - } - - if (m_pc2 != nullptr) - { - m_pc2->Unadvise(m_dwCookie_INetworkEvents); - m_pc2 = nullptr; - } - - if (m_pc3 != nullptr) - { - m_pc3->Unadvise(m_dwCookie_INetworkListManagerEvents); - m_pc3 = nullptr; - } - - m_connection_profile.Reset(); - pSink.Reset(); - pCpc.Reset(); - networkInfoStats.Reset(); - if (pNlm != nullptr) - { - LOG_TRACE("release network list manager..."); - pNlm->Release(); - pNlm = nullptr; - }; } /// - /// Register for COM events and block-wait in RegisterAndListen + /// Register for Windows Runtime events and block-wait in RegisterAndListen /// #pragma warning( push ) -#pragma warning(disable:28159) -#pragma warning(disable:4996) #pragma warning(disable:6320) -// We must use GetVersionEx to retain backwards compat with Win 7 SP1 void NetworkDetector::run() { - // Check Windows version and if below Windows 8, then avoid running Network cost detection logic - OSVERSIONINFO osvi; - BOOL bIsWindows8orLater; - ZeroMemory(&osvi, sizeof(OSVERSIONINFO)); - osvi.dwOSVersionInfoSize = sizeof(OSVERSIONINFO); - GetVersionEx(&osvi); - bIsWindows8orLater = ((osvi.dwMajorVersion >= 6) && (osvi.dwMinorVersion >= 2)) || (osvi.dwMajorVersion > 6); - // Applications not manifested for Windows 8.1 or Windows 10 will return the Windows 8 OS version value (6.2) - if (!bIsWindows8orLater) - { - LOG_INFO("Running on Windows %d.%d without network detector...", osvi.dwMajorVersion, osvi.dwMinorVersion); - return; - } + bool isCoInitialized = false; __try { @@ -490,13 +218,12 @@ namespace MAT_NS_BEGIN } __except (EXCEPTION_EXECUTE_HANDLER) { - LOG_ERROR("Handled exception in network cost detection (Windows 7?)"); + LOG_ERROR("Handled exception in Windows Runtime network cost detection."); } if (isCoInitialized) { CoUninitialize(); - isCoInitialized = false; } } @@ -540,12 +267,12 @@ namespace MAT_NS_BEGIN LOG_TRACE("NetworkDetector is starting..."); { std::unique_lock lock(m_lock); - // Wait for up to NETDETECTOR_COM_SETTLE_MS ms until: - // - COM object is ready; OR - // - COM object can't be started (pre-Win 8 scenario) + // Wait for up to NETDETECTOR_START_TIMEOUT_MS ms until: + // - the listener is subscribed; OR + // - Windows Runtime network information is unavailable int retry = 1; constexpr int max_retries = 2; - while (isRunning && cv.wait_for(lock, std::chrono::milliseconds(NETDETECTOR_COM_SETTLE_MS)) + while (isRunning && cv.wait_for(lock, std::chrono::milliseconds(NETDETECTOR_START_TIMEOUT_MS)) == std::cv_status::timeout && (retry < max_retries)) { LOG_TRACE("NetworkDetector starting up... [%u]", retry); @@ -610,137 +337,6 @@ namespace MAT_NS_BEGIN LOG_TRACE("NetworkDetector done tid=%p", m_listener_tid); } - /// - /// Get network cost name - /// - /// - /// - const char* NetworkDetector::GetNetworkCostName(NetworkCostType type) - { - switch (type) { - case NetworkCostType_Unrestricted: - return "Unrestricted"; - case NetworkCostType_Fixed: - return "Fixed"; - case NetworkCostType_Variable: - return "Variable"; - case NetworkCostType_Unknown: - default: - return "Unknown"; - } - } - - const std::map& NetworkDetector::GetNetworksConnectivity() - { - return m_networks_connectivity; - } - - const std::map& NetworkDetector::GetConnectionsConnectivity() - { - return m_connections_connectivity; - } - - /// - /// Obtain various details about network stack - /// - void NetworkDetector::GetNetworkDetails() - { - LOG_TRACE("Getting network details..."); - ComPtr> hostNames; - HRESULT hr = networkInfoStats->GetHostNames(&hostNames); - if ((!SUCCEEDED(hr))||(!hostNames)) - return; - - m_hostnames.clear(); - unsigned int hostNameCount; - hr = hostNames->get_Size(&hostNameCount); - if (!SUCCEEDED(hr)) - return; - for (unsigned i = 0; i < hostNameCount; ++i) { - MATW::HostNameInfo hostInfo; - ComPtr hostName; - hr = hostNames->GetAt(i, &hostName); - if (!SUCCEEDED(hr)) - continue; - HString rawName; - hostName->get_RawName(rawName.GetAddressOf()); - LOG_TRACE("RawName: %s", to_string(&rawName).c_str()); - - HostNameType type; - hr = hostName->get_Type(&type); - if (!SUCCEEDED(hr)) - continue; - LOG_TRACE("HostNameType: %d", type); - - if (type == HostNameType_DomainName) - continue; - - ComPtr ipInformation; - hr = hostName->get_IPInformation(&ipInformation); - if (!SUCCEEDED(hr)) - continue; - - ComPtr currentAdapter; - hr = ipInformation->get_NetworkAdapter(¤tAdapter); - if (!SUCCEEDED(hr)) - continue; - hr = currentAdapter->get_NetworkAdapterId(&hostInfo.adapterId); - if (!SUCCEEDED(hr)) - continue; - LOG_TRACE("CurrentAdapterId: %s", to_string(hostInfo.adapterId).c_str()); - - ComPtr> prefixLengthReference; - hr = ipInformation->get_PrefixLength(&prefixLengthReference); - if (!SUCCEEDED(hr)) - continue; - hr = prefixLengthReference->get_Value(&hostInfo.prefixLength); - if (!SUCCEEDED(hr)) - continue; - LOG_TRACE("PrefixLength: %d", hostInfo.prefixLength); - - // invalid prefixes - if ((type == HostNameType_Ipv4 && hostInfo.prefixLength > 32) - || (type == HostNameType_Ipv6 && hostInfo.prefixLength > 128)) - continue; - - HString name; - hr = hostName->get_CanonicalName(name.GetAddressOf()); - if (!SUCCEEDED(hr)) - continue; - hostInfo.address = to_string(&name); - LOG_TRACE("CanonicalName: %s", hostInfo.address.c_str()); - - m_hostnames.push_back(hostInfo); - } - - // hr = networkInfoStats->GetInternetConnectionProfile(&m_connection_profile); - // auto profile0 = m_connection_profile.Get(); - - ComPtr> m_connection_profiles; - hr = networkInfoStats->GetConnectionProfiles(&m_connection_profiles); - if (!SUCCEEDED(hr)) - return; - - unsigned int size; - hr = m_connection_profiles->get_Size(&size); - if (!SUCCEEDED(hr)) - return; - - for (unsigned int i = 0; i < size; ++i) { - ComPtr profile; - hr = m_connection_profiles->GetAt(i, &profile); - if (!SUCCEEDED(hr)) - continue; - auto prof = profile.Get(); - HString name; - hr = prof->get_ProfileName(name.GetAddressOf()); - if (!SUCCEEDED(hr)) - continue; - LOG_TRACE("Profile[%d]: name = %s", i, to_string(&name).c_str()); - LOG_TRACE("Profile[%d]: guid = %s", i, GetAdapterId(prof).c_str()); - } - } - } // ::Windows } MAT_NS_END diff --git a/lib/pal/desktop/NetworkDetector.hpp b/lib/pal/desktop/NetworkDetector.hpp index c3eae33df..9ee0c064e 100644 --- a/lib/pal/desktop/NetworkDetector.hpp +++ b/lib/pal/desktop/NetworkDetector.hpp @@ -7,8 +7,6 @@ #include "mat/config.h" #ifdef HAVE_MAT_NETDETECT -#pragma once - // Including SDKDDKVer.h defines the highest available Windows platform. // If you wish to build your application for a previous Windows platform, include WinSDKVer.h and @@ -16,77 +14,28 @@ #include -//#ifndef WIN32_LEAN_AND_MEAN -//#define WIN32_LEAN_AND_MEAN 1 -//#endif - -#pragma once - #include #include #include -#include -#include #include -#include -#include -#include - -#include -#include -#include -#include - -#include -#include -#include -#include #include -#include +#include +#include #include "Enums.hpp" -// #include - using namespace Microsoft::WRL; using namespace Microsoft::WRL::Wrappers; using namespace ABI::Windows::Foundation; -using namespace ABI::Windows::Foundation::Collections; -using namespace ABI::Windows::Networking; using namespace ABI::Windows::Networking::Connectivity; -using namespace std; - namespace MAT_NS_BEGIN { namespace Windows { - /// - /// Host name information structure - /// - struct HostNameInfo { - GUID adapterId; - unsigned char prefixLength; - std::string address; - }; - - /// - /// Convert HString to std::string - /// - /// - /// - std::string to_string(HString *name); - - /// - /// Convert GUID to std::string - /// - /// - /// - std::string to_string(GUID guid); - - class NetworkDetector: public INetworkEvents, INetworkConnectionEvents, INetworkListManagerEvents { + class NetworkDetector { private: @@ -98,16 +47,6 @@ namespace MAT_NS_BEGIN EventRegistrationToken networkStatusChangedToken{}; - /// - /// Current connection profile - /// - ComPtr m_connection_profile; - - /// - /// COM INetworkListManager - /// - INetworkListManager* pNlm; - /// /// Obtain network cost RO. This function does not handle potential exceptions and must only be called from GetNetworkCost() /// @@ -123,7 +62,6 @@ namespace MAT_NS_BEGIN std::mutex m_lock; std::condition_variable cv; bool isRunning = false; - bool isCoInitialized = false; std::thread netDetectThread; /// @@ -131,19 +69,8 @@ namespace MAT_NS_BEGIN /// void run(); - ComPtr pSink; - ComPtr pCpc; - ComPtr m_pc1, m_pc2, m_pc3; - - ULONG m_lRef; - - DWORD m_dwCookie_INetworkEvents; - DWORD m_dwCookie_INetworkConnectionEvents; - DWORD m_dwCookie_INetworkListManagerEvents; DWORD m_listener_tid = 0; - NLM_CONNECTIVITY m_connectivity; - /// /// Register and listen to network state notifications /// @@ -155,12 +82,7 @@ namespace MAT_NS_BEGIN /// void Reset(); - std::vector m_networks; - std::map m_networks_connectivity; - std::map m_connections_connectivity; - std::vector m_hostnames; - int m_currentNetworkCost; - std::atomic m_currentNetworkType; + NetworkCost m_currentNetworkCost = NetworkCost_Unknown; public: @@ -172,24 +94,7 @@ namespace MAT_NS_BEGIN /// /// Createa network status listener /// - NetworkDetector() : - pNlm(nullptr), - networkInfoStats(nullptr), - m_connection_profile(nullptr), - pSink(nullptr), - pCpc(nullptr), - m_pc1(nullptr), - m_pc2(nullptr), - m_pc3(nullptr), - isRunning(false), - isCoInitialized(false), - m_dwCookie_INetworkEvents(0), - m_dwCookie_INetworkConnectionEvents(0), - m_dwCookie_INetworkListManagerEvents(0), - m_currentNetworkCost(0), - m_currentNetworkType(NetworkType_Unknown), - m_listener_tid(0) - {}; + NetworkDetector() = default; /// /// @@ -205,14 +110,7 @@ namespace MAT_NS_BEGIN /// /// /// - virtual ~NetworkDetector(); - - /// - /// - /// - /// - /// - const char *GetNetworkCostName(NetworkCostType type); + ~NetworkDetector(); /// /// Get current network cost @@ -226,48 +124,6 @@ namespace MAT_NS_BEGIN /// NetworkCost const& GetNetworkCost(); - /// - /// Get last cached network type - /// - /// - NetworkType GetNetworkType(); - - /// - /// Get adapter ID for connection profile - /// - /// - /// - std::string GetAdapterId(IConnectionProfile *profile); - - int GetConnectivity(); - - const std::map& GetNetworksConnectivity(); - const std::map& GetConnectionsConnectivity(); - - void GetNetworkDetails(); - - IConnectionProfile* GetCurrentConnectionProfile() - { - return m_connection_profile.Get(); - } - - public: - - // Inherited via INetworkListManagerEvents - virtual HRESULT STDMETHODCALLTYPE QueryInterface(REFIID riid, void ** ppvObject) noexcept override; - virtual ULONG STDMETHODCALLTYPE AddRef(void) noexcept override; - virtual ULONG STDMETHODCALLTYPE Release(void) noexcept override; - virtual HRESULT STDMETHODCALLTYPE ConnectivityChanged(NLM_CONNECTIVITY newConnectivity) override; - - // Inherited via INetworkEvents - virtual HRESULT STDMETHODCALLTYPE NetworkAdded(GUID networkId) override; - virtual HRESULT STDMETHODCALLTYPE NetworkDeleted(GUID networkId) override; - virtual HRESULT STDMETHODCALLTYPE NetworkConnectivityChanged(GUID networkId, NLM_CONNECTIVITY newConnectivity) override; - virtual HRESULT STDMETHODCALLTYPE NetworkPropertyChanged(GUID networkId, NLM_NETWORK_PROPERTY_CHANGE flags) override; - - // Inherited via INetworkConnectionEvents - virtual HRESULT STDMETHODCALLTYPE NetworkConnectionConnectivityChanged(GUID connectionId, NLM_CONNECTIVITY newConnectivity) override; - virtual HRESULT STDMETHODCALLTYPE NetworkConnectionPropertyChanged(GUID connectionId, NLM_CONNECTION_PROPERTY_CHANGE flags) override; }; } diff --git a/lib/pal/desktop/WindowsDesktopNetworkInformationImpl.cpp b/lib/pal/desktop/WindowsDesktopNetworkInformationImpl.cpp index df388c7ea..d3082a313 100644 --- a/lib/pal/desktop/WindowsDesktopNetworkInformationImpl.cpp +++ b/lib/pal/desktop/WindowsDesktopNetworkInformationImpl.cpp @@ -89,7 +89,6 @@ namespace PAL_NS_BEGIN { #ifdef HAVE_MAT_NETDETECT if (m_isNetDetectEnabled) { networkDetector = std::unique_ptr(new MATW::NetworkDetector()); - networkDetector->AddRef(); networkDetector->Start(); } #endif @@ -101,7 +100,6 @@ namespace PAL_NS_BEGIN { #ifdef HAVE_MAT_NETDETECT if (m_isNetDetectEnabled) { networkDetector->Stop(); - networkDetector->Release(); } #endif } @@ -112,4 +110,3 @@ namespace PAL_NS_BEGIN { } } PAL_NS_END #endif - From 6c979a22c39ca046e0a79e4a00f309bd3dbfd629 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 18 Sep 2026 05:37:12 -0500 Subject: [PATCH 10/23] Run leak analysis only after relevant changes Avoid spending hosted runner time when main has not changed while preserving manual analysis on demand. Files changed: - .github/workflows/memory-leak-analysis.yml Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- .github/workflows/memory-leak-analysis.yml | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/.github/workflows/memory-leak-analysis.yml b/.github/workflows/memory-leak-analysis.yml index a7e82c300..0bced86b8 100644 --- a/.github/workflows/memory-leak-analysis.yml +++ b/.github/workflows/memory-leak-analysis.yml @@ -1,15 +1,31 @@ -name: Periodic memory leak analysis +name: Memory leak analysis on: workflow_dispatch: + push: + branches: + - main + paths: + - .github/scripts/run-drmemory.ps1 + - .github/workflows/memory-leak-analysis.yml + - CMakeLists.txt + - CMakePresets.json + - Solutions/** + - cmake/** + - examples/cpp/SampleCppMini/** + - lib/** + - sqlite/** + - tests/** + - third_party/Solutions/zlib/** + - third_party/googletest + - tools/gen-version.cmd + - zlib/** pull_request: branches: - main paths: - .github/scripts/run-drmemory.ps1 - .github/workflows/memory-leak-analysis.yml - schedule: - - cron: 0 5 * * 1 permissions: contents: read From a266054a666382cb21aa95721267046ae385ef29 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 18 Sep 2026 13:12:39 -0500 Subject: [PATCH 11/23] Address Copilot leak-review findings Restore approaching-data-limit handling in the WinRT cost mapping, verified in lib/pal/desktop/NetworkDetector.cpp. Check netprofm.dll while the detector is active, verified in tests/unittests/NetworkDetectorTests.cpp. Include APITest.C_API_Test in Linux leak analysis after confirming the test passes on Linux, verified in .github/workflows/memory-leak-analysis.yml. Files changed: - .github/workflows/memory-leak-analysis.yml - lib/pal/desktop/NetworkDetector.cpp - tests/unittests/NetworkDetectorTests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- .github/workflows/memory-leak-analysis.yml | 2 +- lib/pal/desktop/NetworkDetector.cpp | 4 +++- tests/unittests/NetworkDetectorTests.cpp | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/memory-leak-analysis.yml b/.github/workflows/memory-leak-analysis.yml index 0bced86b8..f7e76d7cf 100644 --- a/.github/workflows/memory-leak-analysis.yml +++ b/.github/workflows/memory-leak-analysis.yml @@ -189,7 +189,7 @@ jobs: -LogDirectory drmemory-results -Scenario functional-tests -TargetPath out/tests/functests/FuncTests - -TargetArguments "--gtest_filter=-APITest.C_API_Test:BasicFuncTests.killSwitchWorks" + -TargetArguments "--gtest_filter=-BasicFuncTests.killSwitchWorks" - name: Analyze basic sample shell: pwsh diff --git a/lib/pal/desktop/NetworkDetector.cpp b/lib/pal/desktop/NetworkDetector.cpp index caa7e55a4..7d41cb5b6 100644 --- a/lib/pal/desktop/NetworkDetector.cpp +++ b/lib/pal/desktop/NetworkDetector.cpp @@ -92,14 +92,16 @@ namespace MAT_NS_BEGIN boolean roaming = false; boolean overDataLimit = false; + boolean approachingDataLimit = false; NetworkCostType costType = NetworkCostType_Unknown; if (FAILED(connectionCost->get_Roaming(&roaming)) || FAILED(connectionCost->get_OverDataLimit(&overDataLimit)) || + FAILED(connectionCost->get_ApproachingDataLimit(&approachingDataLimit)) || FAILED(connectionCost->get_NetworkCostType(&costType))) { return result; } - if (roaming || overDataLimit) { + if (roaming || overDataLimit || approachingDataLimit) { return NetworkCost_Roaming; } diff --git a/tests/unittests/NetworkDetectorTests.cpp b/tests/unittests/NetworkDetectorTests.cpp index 84d08092d..2110d7048 100644 --- a/tests/unittests/NetworkDetectorTests.cpp +++ b/tests/unittests/NetworkDetectorTests.cpp @@ -15,6 +15,7 @@ TEST(NetworkDetectorTests, StartsReadsCostAndStopsWithoutNetworkListManager) MATW::NetworkDetector detector; ASSERT_TRUE(detector.Start()); EXPECT_TRUE(detector.isUp()); + EXPECT_EQ(GetModuleHandleW(L"netprofm.dll"), nullptr); const auto cost = detector.GetCurrentNetworkCost(); EXPECT_THAT(cost, AnyOf( From 3192701b079f3e574dbb2241f3820ddb4c0bcae6 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 18 Sep 2026 13:28:53 -0500 Subject: [PATCH 12/23] Synchronize cached network detector state Store network cost and running state atomically so WinRT callbacks cannot race caller reads. Return the cached cost by value instead of exposing a concurrently updated reference. Verified at: - lib/pal/desktop/NetworkDetector.cpp - lib/pal/desktop/NetworkDetector.hpp Files changed: - lib/pal/desktop/NetworkDetector.cpp - lib/pal/desktop/NetworkDetector.hpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- lib/pal/desktop/NetworkDetector.cpp | 22 +++++++++++++--------- lib/pal/desktop/NetworkDetector.hpp | 9 +++++---- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/lib/pal/desktop/NetworkDetector.cpp b/lib/pal/desktop/NetworkDetector.cpp index 7d41cb5b6..0559f545a 100644 --- a/lib/pal/desktop/NetworkDetector.cpp +++ b/lib/pal/desktop/NetworkDetector.cpp @@ -21,8 +21,8 @@ namespace MAT_NS_BEGIN { namespace Windows { - NetworkCost const& NetworkDetector::GetNetworkCost() { - return m_currentNetworkCost; + NetworkCost NetworkDetector::GetNetworkCost() { + return m_currentNetworkCost.load(std::memory_order_relaxed); } /// @@ -34,9 +34,9 @@ namespace MAT_NS_BEGIN #pragma warning(disable: 6320) int NetworkDetector::GetCurrentNetworkCost() { - m_currentNetworkCost = NetworkCost_Unknown; + NetworkCost currentNetworkCost = NetworkCost_Unknown; __try { - m_currentNetworkCost = _GetCurrentNetworkCost(); + currentNetworkCost = _GetCurrentNetworkCost(); } //****************************************************************************************************************************** // This code is required as a workaround for an issue in Visual Studio debug host mode: crash in W.N.C.dll @@ -50,17 +50,18 @@ namespace MAT_NS_BEGIN __except (EXCEPTION_EXECUTE_HANDLER) { LOG_ERROR("Unable to obtain network state!"); - m_currentNetworkCost = NetworkCost_Unknown; } + m_currentNetworkCost.store(currentNetworkCost, std::memory_order_relaxed); + // Notify the app about current network cost change DebugEvent evt; evt.type = DebugEventType::EVT_NET_CHANGED; - evt.param1 = m_currentNetworkCost; + evt.param1 = currentNetworkCost; evt.param2 = false; ILogManager::DispatchEventBroadcast(evt); - return m_currentNetworkCost; + return currentNetworkCost; } #pragma warning(pop) @@ -280,7 +281,10 @@ namespace MAT_NS_BEGIN LOG_TRACE("NetworkDetector starting up... [%u]", retry); retry++; } - LOG_TRACE("NetworkDetector tid=%p running=%u", m_listener_tid, isRunning); + LOG_TRACE( + "NetworkDetector tid=%p running=%u", + m_listener_tid, + isRunning.load(std::memory_order_relaxed)); } } else @@ -290,7 +294,7 @@ namespace MAT_NS_BEGIN isRunning = false; } - return isRunning; + return isRunning.load(std::memory_order_relaxed); }; /// diff --git a/lib/pal/desktop/NetworkDetector.hpp b/lib/pal/desktop/NetworkDetector.hpp index 9ee0c064e..075353fd7 100644 --- a/lib/pal/desktop/NetworkDetector.hpp +++ b/lib/pal/desktop/NetworkDetector.hpp @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -61,7 +62,7 @@ namespace MAT_NS_BEGIN std::mutex m_lock; std::condition_variable cv; - bool isRunning = false; + std::atomic isRunning{ false }; std::thread netDetectThread; /// @@ -82,14 +83,14 @@ namespace MAT_NS_BEGIN /// void Reset(); - NetworkCost m_currentNetworkCost = NetworkCost_Unknown; + std::atomic m_currentNetworkCost{ NetworkCost_Unknown }; public: /// /// /// - bool isUp() { return isRunning; }; + bool isUp() { return isRunning.load(std::memory_order_relaxed); }; /// /// Createa network status listener @@ -122,7 +123,7 @@ namespace MAT_NS_BEGIN /// Get last cached network cost /// /// - NetworkCost const& GetNetworkCost(); + NetworkCost GetNetworkCost(); }; From 88c733881f43e3fab7548b3a9caaac9f0d0703eb Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 18 Sep 2026 13:42:29 -0500 Subject: [PATCH 13/23] Cover WinRT network cost mapping Remove the ignored MATSDK_USE_WININET build property so leak analysis does not imply a transport selection it never made. Extract and test WinRT cost mapping for unrestricted, metered, roaming, over-limit, and approaching-limit states, and verify synchronous refresh updates the cache. Verified at: - .github/workflows/memory-leak-analysis.yml - lib/pal/desktop/NetworkDetector.cpp - tests/unittests/NetworkDetectorTests.cpp Files changed: - .github/workflows/memory-leak-analysis.yml - lib/pal/desktop/NetworkDetector.cpp - lib/pal/desktop/NetworkDetector.hpp - tests/unittests/NetworkDetectorTests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- .github/workflows/memory-leak-analysis.yml | 1 - lib/pal/desktop/NetworkDetector.cpp | 41 ++++++++++++---------- lib/pal/desktop/NetworkDetector.hpp | 6 ++++ tests/unittests/NetworkDetectorTests.cpp | 16 +++++++++ 4 files changed, 45 insertions(+), 19 deletions(-) diff --git a/.github/workflows/memory-leak-analysis.yml b/.github/workflows/memory-leak-analysis.yml index f7e76d7cf..bbb0ee88e 100644 --- a/.github/workflows/memory-leak-analysis.yml +++ b/.github/workflows/memory-leak-analysis.yml @@ -64,7 +64,6 @@ jobs: /p:BuildProjectReferences=true /p:Configuration=Debug /p:Platform=x64 - /p:MATSDK_USE_WININET=false /maxcpucount:2 - name: Download Dr. Memory diff --git a/lib/pal/desktop/NetworkDetector.cpp b/lib/pal/desktop/NetworkDetector.cpp index 0559f545a..4b13e0216 100644 --- a/lib/pal/desktop/NetworkDetector.cpp +++ b/lib/pal/desktop/NetworkDetector.cpp @@ -21,6 +21,28 @@ namespace MAT_NS_BEGIN { namespace Windows { + NetworkCost MapNetworkCost( + NetworkCostType costType, + boolean roaming, + boolean overDataLimit, + boolean approachingDataLimit) + { + if (roaming || overDataLimit || approachingDataLimit) { + return NetworkCost_Roaming; + } + + switch (costType) { + case NetworkCostType_Unrestricted: + return NetworkCost_Unmetered; + case NetworkCostType_Fixed: + case NetworkCostType_Variable: + return NetworkCost_Metered; + case NetworkCostType_Unknown: + default: + return NetworkCost_Unknown; + } + } + NetworkCost NetworkDetector::GetNetworkCost() { return m_currentNetworkCost.load(std::memory_order_relaxed); } @@ -102,24 +124,7 @@ namespace MAT_NS_BEGIN return result; } - if (roaming || overDataLimit || approachingDataLimit) { - return NetworkCost_Roaming; - } - - switch (costType) { - case NetworkCostType_Unrestricted: - result = NetworkCost_Unmetered; - break; - case NetworkCostType_Fixed: - case NetworkCostType_Variable: - result = NetworkCost_Metered; - break; - case NetworkCostType_Unknown: - default: - break; - } - - return result; + return MapNetworkCost(costType, roaming, overDataLimit, approachingDataLimit); } /// diff --git a/lib/pal/desktop/NetworkDetector.hpp b/lib/pal/desktop/NetworkDetector.hpp index 075353fd7..550cac33f 100644 --- a/lib/pal/desktop/NetworkDetector.hpp +++ b/lib/pal/desktop/NetworkDetector.hpp @@ -36,6 +36,12 @@ namespace MAT_NS_BEGIN { namespace Windows { + NetworkCost MapNetworkCost( + NetworkCostType costType, + boolean roaming, + boolean overDataLimit, + boolean approachingDataLimit); + class NetworkDetector { private: diff --git a/tests/unittests/NetworkDetectorTests.cpp b/tests/unittests/NetworkDetectorTests.cpp index 2110d7048..b8d71bc94 100644 --- a/tests/unittests/NetworkDetectorTests.cpp +++ b/tests/unittests/NetworkDetectorTests.cpp @@ -8,6 +8,21 @@ using namespace MAT; using namespace testing; +TEST(NetworkDetectorTests, MapsWinRTNetworkCosts) +{ + EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unrestricted, false, false, false), NetworkCost_Unmetered); + EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Fixed, false, false, false), NetworkCost_Metered); + EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Variable, false, false, false), NetworkCost_Metered); + EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unknown, false, false, false), NetworkCost_Unknown); +} + +TEST(NetworkDetectorTests, MapsRestrictiveWinRTNetworkStates) +{ + EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unrestricted, true, false, false), NetworkCost_Roaming); + EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unrestricted, false, true, false), NetworkCost_Roaming); + EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unrestricted, false, false, true), NetworkCost_Roaming); +} + TEST(NetworkDetectorTests, StartsReadsCostAndStopsWithoutNetworkListManager) { ASSERT_EQ(GetModuleHandleW(L"netprofm.dll"), nullptr); @@ -23,6 +38,7 @@ TEST(NetworkDetectorTests, StartsReadsCostAndStopsWithoutNetworkListManager) Eq(NetworkCost_Unmetered), Eq(NetworkCost_Metered), Eq(NetworkCost_Roaming))); + EXPECT_EQ(detector.GetNetworkCost(), cost); detector.Stop(); EXPECT_FALSE(detector.isUp()); From 355b250ffc62c3b865e19412daf4b36ba382677b Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 18 Sep 2026 13:58:38 -0500 Subject: [PATCH 14/23] Synchronize WinRT callback teardown Keep per-subscription callback state alive independently, reject callbacks after shutdown starts, and wait for active callbacks before releasing detector resources. Inspect Dr. Memory global module logs for netprofm.dll and require logs for every Windows scenario so the regression gate cannot pass vacuously. Verified at: - lib/pal/desktop/NetworkDetector.cpp - .github/workflows/memory-leak-analysis.yml Files changed: - .github/workflows/memory-leak-analysis.yml - lib/pal/desktop/NetworkDetector.cpp - lib/pal/desktop/NetworkDetector.hpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- .github/workflows/memory-leak-analysis.yml | 11 +++- lib/pal/desktop/NetworkDetector.cpp | 76 +++++++++++++++++++++- lib/pal/desktop/NetworkDetector.hpp | 4 ++ 3 files changed, 87 insertions(+), 4 deletions(-) diff --git a/.github/workflows/memory-leak-analysis.yml b/.github/workflows/memory-leak-analysis.yml index bbb0ee88e..fe57a8ebd 100644 --- a/.github/workflows/memory-leak-analysis.yml +++ b/.github/workflows/memory-leak-analysis.yml @@ -111,8 +111,15 @@ jobs: - name: Verify Network List Manager is not loaded shell: pwsh run: | - $matches = Get-ChildItem drmemory-results -Filter *.txt -File -Recurse | - Select-String -Pattern 'netprofm\.dll' + $moduleLogs = @() + foreach ($scenario in @("unit-tests", "functional-tests", "sample-cpp-mini")) { + $scenarioLogs = @(Get-ChildItem "drmemory-results/$scenario" -Filter global.*.log -File -Recurse) + if ($scenarioLogs.Count -eq 0) { + throw "Dr. Memory did not produce a module log for $scenario." + } + $moduleLogs += $scenarioLogs + } + $matches = $moduleLogs | Select-String -Pattern 'module load event:\s+"netprofm\.dll"' if ($matches) { $matches | ForEach-Object { Write-Error "$($_.Path):$($_.LineNumber): $($_.Line)" } throw "Network detection loaded netprofm.dll." diff --git a/lib/pal/desktop/NetworkDetector.cpp b/lib/pal/desktop/NetworkDetector.cpp index 4b13e0216..b18b57bd5 100644 --- a/lib/pal/desktop/NetworkDetector.cpp +++ b/lib/pal/desktop/NetworkDetector.cpp @@ -21,6 +21,64 @@ namespace MAT_NS_BEGIN { namespace Windows { + struct NetworkDetector::CallbackState { + explicit CallbackState(NetworkDetector& owner) : + detector(&owner) + { + } + + class Invocation { + public: + explicit Invocation(std::shared_ptr state) : + callbackState(state) + { + std::lock_guard lock(callbackState->mutex); + if (callbackState->acceptCallbacks) { + detector = callbackState->detector; + ++callbackState->activeCallbacks; + } + } + + ~Invocation() + { + if (detector != nullptr) { + std::lock_guard lock(callbackState->mutex); + --callbackState->activeCallbacks; + callbackState->cv.notify_all(); + } + } + + Invocation(Invocation const&) = delete; + Invocation& operator=(Invocation const&) = delete; + + NetworkDetector* GetDetector() const + { + return detector; + } + + private: + std::shared_ptr callbackState; + NetworkDetector* detector = nullptr; + }; + + void StopAndWait() + { + std::unique_lock lock(mutex); + acceptCallbacks = false; + cv.wait(lock, [this]() { return activeCallbacks == 0; }); + detector = nullptr; + } + + private: + std::mutex mutex; + std::condition_variable cv; + NetworkDetector* detector; + size_t activeCallbacks = 0; + bool acceptCallbacks = true; + + friend class Invocation; + }; + NetworkCost MapNetworkCost( NetworkCostType costType, boolean roaming, @@ -144,13 +202,20 @@ namespace MAT_NS_BEGIN bool NetworkDetector::RegisterAndListen() noexcept { + networkStatusCallbackState = std::make_shared(*this); + const auto callbackState = networkStatusCallbackState; networkStatusChangedHandler = Callback( - [this](IInspectable*) -> HRESULT { - GetCurrentNetworkCost(); + [callbackState](IInspectable*) -> HRESULT { + CallbackState::Invocation invocation(callbackState); + if (auto detector = invocation.GetDetector()) { + detector->GetCurrentNetworkCost(); + } return S_OK; }); if (networkStatusChangedHandler == nullptr) { LOG_ERROR("Unable to create network status handler."); + networkStatusCallbackState->StopAndWait(); + networkStatusCallbackState.reset(); return false; } @@ -159,6 +224,8 @@ namespace MAT_NS_BEGIN &networkStatusChangedToken); if (FAILED(hr)) { LOG_ERROR("Unable to subscribe to network status changes."); + networkStatusCallbackState->StopAndWait(); + networkStatusCallbackState.reset(); networkStatusChangedHandler.Reset(); return false; } @@ -188,12 +255,17 @@ namespace MAT_NS_BEGIN /// void NetworkDetector::Reset() { + if (networkStatusCallbackState != nullptr) + { + networkStatusCallbackState->StopAndWait(); + } if (networkStatusChangedToken.value != 0 && networkInfoStats != nullptr) { networkInfoStats->remove_NetworkStatusChanged(networkStatusChangedToken); networkStatusChangedToken.value = 0; } networkStatusChangedHandler.Reset(); + networkStatusCallbackState.reset(); networkInfoStats.Reset(); } diff --git a/lib/pal/desktop/NetworkDetector.hpp b/lib/pal/desktop/NetworkDetector.hpp index 550cac33f..9b8e75c4f 100644 --- a/lib/pal/desktop/NetworkDetector.hpp +++ b/lib/pal/desktop/NetworkDetector.hpp @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -46,12 +47,15 @@ namespace MAT_NS_BEGIN private: + struct CallbackState; + /// /// Current network info stats /// ComPtr networkInfoStats; ComPtr networkStatusChangedHandler; EventRegistrationToken networkStatusChangedToken{}; + std::shared_ptr networkStatusCallbackState; /// From 68f5276847cbbf98b3e540f3159413eaaf77339d Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 18 Sep 2026 14:16:51 -0500 Subject: [PATCH 15/23] Decouple WinRT callbacks from detector lifetime Capture COM and atomic cache state by shared ownership so an in-flight notification never dereferences a destroyed detector and teardown does not wait on a callback that can synchronously initiate shutdown. Run Reset from an SEH finally path so subscriptions are disabled and released even when Windows Runtime raises a structured exception. Verified at: - lib/pal/desktop/NetworkDetector.cpp - lib/pal/desktop/NetworkDetector.hpp Files changed: - lib/pal/desktop/NetworkDetector.cpp - lib/pal/desktop/NetworkDetector.hpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- lib/pal/desktop/NetworkDetector.cpp | 195 +++++++++++----------------- lib/pal/desktop/NetworkDetector.hpp | 10 +- 2 files changed, 78 insertions(+), 127 deletions(-) diff --git a/lib/pal/desktop/NetworkDetector.cpp b/lib/pal/desktop/NetworkDetector.cpp index b18b57bd5..9e3a7e747 100644 --- a/lib/pal/desktop/NetworkDetector.cpp +++ b/lib/pal/desktop/NetworkDetector.cpp @@ -22,61 +22,7 @@ namespace MAT_NS_BEGIN namespace Windows { struct NetworkDetector::CallbackState { - explicit CallbackState(NetworkDetector& owner) : - detector(&owner) - { - } - - class Invocation { - public: - explicit Invocation(std::shared_ptr state) : - callbackState(state) - { - std::lock_guard lock(callbackState->mutex); - if (callbackState->acceptCallbacks) { - detector = callbackState->detector; - ++callbackState->activeCallbacks; - } - } - - ~Invocation() - { - if (detector != nullptr) { - std::lock_guard lock(callbackState->mutex); - --callbackState->activeCallbacks; - callbackState->cv.notify_all(); - } - } - - Invocation(Invocation const&) = delete; - Invocation& operator=(Invocation const&) = delete; - - NetworkDetector* GetDetector() const - { - return detector; - } - - private: - std::shared_ptr callbackState; - NetworkDetector* detector = nullptr; - }; - - void StopAndWait() - { - std::unique_lock lock(mutex); - acceptCallbacks = false; - cv.wait(lock, [this]() { return activeCallbacks == 0; }); - detector = nullptr; - } - - private: - std::mutex mutex; - std::condition_variable cv; - NetworkDetector* detector; - size_t activeCallbacks = 0; - bool acceptCallbacks = true; - - friend class Invocation; + std::atomic acceptCallbacks{ true }; }; NetworkCost MapNetworkCost( @@ -101,22 +47,55 @@ namespace MAT_NS_BEGIN } } - NetworkCost NetworkDetector::GetNetworkCost() { - return m_currentNetworkCost.load(std::memory_order_relaxed); + static NetworkCost QueryCurrentNetworkCost(INetworkInformationStatics* networkInfoStats) + { + NetworkCost result = NetworkCost_Unknown; + LOG_TRACE("get network cost...\n"); + + if (networkInfoStats == nullptr) { + LOG_WARN("Windows network information is unavailable!"); + return result; + } + + ComPtr connectionProfile; + HRESULT hr = networkInfoStats->GetInternetConnectionProfile(&connectionProfile); + if (FAILED(hr) || connectionProfile == nullptr) { + return result; + } + + ComPtr connectionCost; + hr = connectionProfile->GetConnectionCost(&connectionCost); + if (FAILED(hr) || connectionCost == nullptr) { + return result; + } + + boolean roaming = false; + boolean overDataLimit = false; + boolean approachingDataLimit = false; + NetworkCostType costType = NetworkCostType_Unknown; + if (FAILED(connectionCost->get_Roaming(&roaming)) || + FAILED(connectionCost->get_OverDataLimit(&overDataLimit)) || + FAILED(connectionCost->get_ApproachingDataLimit(&approachingDataLimit)) || + FAILED(connectionCost->get_NetworkCostType(&costType))) { + return result; + } + + return MapNetworkCost(costType, roaming, overDataLimit, approachingDataLimit); } /// /// Get current realtime network cost synchronously. /// This function provides an SEH handler for Windows Runtime failures. /// - /// #pragma warning(push) #pragma warning(disable: 6320) - int NetworkDetector::GetCurrentNetworkCost() + static int RefreshNetworkCost( + INetworkInformationStatics* networkInfoStats, + std::atomic& currentNetworkCostState) { NetworkCost currentNetworkCost = NetworkCost_Unknown; __try { - currentNetworkCost = _GetCurrentNetworkCost(); + currentNetworkCost = QueryCurrentNetworkCost(networkInfoStats); } //****************************************************************************************************************************** // This code is required as a workaround for an issue in Visual Studio debug host mode: crash in W.N.C.dll @@ -132,9 +111,8 @@ namespace MAT_NS_BEGIN LOG_ERROR("Unable to obtain network state!"); } - m_currentNetworkCost.store(currentNetworkCost, std::memory_order_relaxed); + currentNetworkCostState.store(currentNetworkCost, std::memory_order_relaxed); - // Notify the app about current network cost change DebugEvent evt; evt.type = DebugEventType::EVT_NET_CHANGED; evt.param1 = currentNetworkCost; @@ -145,44 +123,13 @@ namespace MAT_NS_BEGIN } #pragma warning(pop) - /// - /// Internal implementation - /// - /// - NetworkCost NetworkDetector::_GetCurrentNetworkCost() - { - NetworkCost result = NetworkCost_Unknown; - LOG_TRACE("get network cost...\n"); - - if (networkInfoStats == nullptr) { - LOG_WARN("Windows network information is unavailable!"); - return result; - } - - ComPtr connectionProfile; - HRESULT hr = networkInfoStats->GetInternetConnectionProfile(&connectionProfile); - if (FAILED(hr) || connectionProfile == nullptr) { - return result; - } - - ComPtr connectionCost; - hr = connectionProfile->GetConnectionCost(&connectionCost); - if (FAILED(hr) || connectionCost == nullptr) { - return result; - } - - boolean roaming = false; - boolean overDataLimit = false; - boolean approachingDataLimit = false; - NetworkCostType costType = NetworkCostType_Unknown; - if (FAILED(connectionCost->get_Roaming(&roaming)) || - FAILED(connectionCost->get_OverDataLimit(&overDataLimit)) || - FAILED(connectionCost->get_ApproachingDataLimit(&approachingDataLimit)) || - FAILED(connectionCost->get_NetworkCostType(&costType))) { - return result; - } + NetworkCost NetworkDetector::GetNetworkCost() { + return m_currentNetworkCost->load(std::memory_order_relaxed); + } - return MapNetworkCost(costType, roaming, overDataLimit, approachingDataLimit); + int NetworkDetector::GetCurrentNetworkCost() + { + return RefreshNetworkCost(networkInfoStats.Get(), *m_currentNetworkCost); } /// @@ -202,19 +149,20 @@ namespace MAT_NS_BEGIN bool NetworkDetector::RegisterAndListen() noexcept { - networkStatusCallbackState = std::make_shared(*this); + networkStatusCallbackState = std::make_shared(); const auto callbackState = networkStatusCallbackState; + const auto currentNetworkCost = m_currentNetworkCost; + const auto networkInformation = networkInfoStats; networkStatusChangedHandler = Callback( - [callbackState](IInspectable*) -> HRESULT { - CallbackState::Invocation invocation(callbackState); - if (auto detector = invocation.GetDetector()) { - detector->GetCurrentNetworkCost(); + [callbackState, currentNetworkCost, networkInformation](IInspectable*) -> HRESULT { + if (callbackState->acceptCallbacks.load(std::memory_order_acquire)) { + RefreshNetworkCost(networkInformation.Get(), *currentNetworkCost); } return S_OK; }); if (networkStatusChangedHandler == nullptr) { LOG_ERROR("Unable to create network status handler."); - networkStatusCallbackState->StopAndWait(); + networkStatusCallbackState->acceptCallbacks.store(false, std::memory_order_release); networkStatusCallbackState.reset(); return false; } @@ -224,7 +172,7 @@ namespace MAT_NS_BEGIN &networkStatusChangedToken); if (FAILED(hr)) { LOG_ERROR("Unable to subscribe to network status changes."); - networkStatusCallbackState->StopAndWait(); + networkStatusCallbackState->acceptCallbacks.store(false, std::memory_order_release); networkStatusCallbackState.reset(); networkStatusChangedHandler.Reset(); return false; @@ -257,15 +205,16 @@ namespace MAT_NS_BEGIN { if (networkStatusCallbackState != nullptr) { - networkStatusCallbackState->StopAndWait(); + networkStatusCallbackState->acceptCallbacks.store(false, std::memory_order_release); } + networkStatusChangedHandler.Reset(); + networkStatusCallbackState.reset(); if (networkStatusChangedToken.value != 0 && networkInfoStats != nullptr) { - networkInfoStats->remove_NetworkStatusChanged(networkStatusChangedToken); + const auto token = networkStatusChangedToken; networkStatusChangedToken.value = 0; + networkInfoStats->remove_NetworkStatusChanged(token); } - networkStatusChangedHandler.Reset(); - networkStatusCallbackState.reset(); networkInfoStats.Reset(); } @@ -280,19 +229,25 @@ namespace MAT_NS_BEGIN __try { - HRESULT hr = CoInitialize(nullptr); - if (FAILED(hr)) + __try { - LOG_ERROR("CoInitialize Failed."); - return; - } + HRESULT hr = CoInitialize(nullptr); + if (FAILED(hr)) + { + LOG_ERROR("CoInitialize Failed."); + return; + } - isCoInitialized = true; - if (GetNetworkInfoStats()) + isCoInitialized = true; + if (GetNetworkInfoStats()) + { + GetCurrentNetworkCost(); + LOG_TRACE("start listening to events..."); + RegisterAndListen(); + } + } + __finally { - GetCurrentNetworkCost(); - LOG_TRACE("start listening to events..."); - RegisterAndListen(); Reset(); } } diff --git a/lib/pal/desktop/NetworkDetector.hpp b/lib/pal/desktop/NetworkDetector.hpp index 9b8e75c4f..c2fad0610 100644 --- a/lib/pal/desktop/NetworkDetector.hpp +++ b/lib/pal/desktop/NetworkDetector.hpp @@ -58,12 +58,6 @@ namespace MAT_NS_BEGIN std::shared_ptr networkStatusCallbackState; - /// - /// Obtain network cost RO. This function does not handle potential exceptions and must only be called from GetNetworkCost() - /// - /// - NetworkCost _GetCurrentNetworkCost(); - /// /// Get instance of network info stats /// @@ -93,7 +87,9 @@ namespace MAT_NS_BEGIN /// void Reset(); - std::atomic m_currentNetworkCost{ NetworkCost_Unknown }; + std::shared_ptr> m_currentNetworkCost{ + std::make_shared>(NetworkCost_Unknown) + }; public: From ffc696e955fb6d8e8417623ff4612349a22e5bed Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 18 Sep 2026 15:14:34 -0500 Subject: [PATCH 16/23] Exclude instrumented SQLite timing assertion Dr. Memory changes execution speed and caused the one-second storage benchmark to fail at 1.384 seconds even though its functional operations succeeded. Keep the benchmark in normal CI while excluding only that wall-clock assertion from leak analysis. Files changed: - .github/workflows/memory-leak-analysis.yml Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- .github/workflows/memory-leak-analysis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/memory-leak-analysis.yml b/.github/workflows/memory-leak-analysis.yml index fe57a8ebd..a1f75e167 100644 --- a/.github/workflows/memory-leak-analysis.yml +++ b/.github/workflows/memory-leak-analysis.yml @@ -88,6 +88,7 @@ jobs: -LogDirectory drmemory-results -Scenario unit-tests -TargetPath Solutions/out/Debug/x64/UnitTests/UnitTests.exe + -TargetArguments "--gtest_filter=-OfflineStorageTests_SQLite.StoreThousandEventsTakesLessThanASecond" - name: Analyze functional tests shell: pwsh From 6da670a6da3d78890dfabbe3644a046a413112e9 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 18 Sep 2026 15:26:53 -0500 Subject: [PATCH 17/23] Initialize the WinRT listener apartment Use RoInitialize with the multithreaded apartment and balance successful initialization with RoUninitialize so Windows Runtime activation is valid on every supported Windows target. Verified at: - lib/pal/desktop/NetworkDetector.cpp Files changed: - lib/pal/desktop/NetworkDetector.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- lib/pal/desktop/NetworkDetector.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/pal/desktop/NetworkDetector.cpp b/lib/pal/desktop/NetworkDetector.cpp index 9e3a7e747..efd888352 100644 --- a/lib/pal/desktop/NetworkDetector.cpp +++ b/lib/pal/desktop/NetworkDetector.cpp @@ -225,20 +225,20 @@ namespace MAT_NS_BEGIN #pragma warning(disable:6320) void NetworkDetector::run() { - bool isCoInitialized = false; + bool isRoInitialized = false; __try { __try { - HRESULT hr = CoInitialize(nullptr); + HRESULT hr = RoInitialize(RO_INIT_MULTITHREADED); if (FAILED(hr)) { - LOG_ERROR("CoInitialize Failed."); + LOG_ERROR("RoInitialize failed."); return; } - isCoInitialized = true; + isRoInitialized = true; if (GetNetworkInfoStats()) { GetCurrentNetworkCost(); @@ -256,9 +256,9 @@ namespace MAT_NS_BEGIN LOG_ERROR("Handled exception in Windows Runtime network cost detection."); } - if (isCoInitialized) + if (isRoInitialized) { - CoUninitialize(); + RoUninitialize(); } } From ce3e63ca6ea1ad5f5fa4996797f59a235e8193cc Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 18 Sep 2026 15:39:37 -0500 Subject: [PATCH 18/23] Guard network detector tests by feature Compile the Windows detector tests only when HAVE_MAT_NETDETECT is enabled so compact custom SKUs that omit the detector still build the test project. Verified at: - tests/unittests/NetworkDetectorTests.cpp - Solutions/build.compact.props Files changed: - tests/unittests/NetworkDetectorTests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: df9f344e-e064-404e-ae6f-c0ef455d747c --- tests/unittests/NetworkDetectorTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unittests/NetworkDetectorTests.cpp b/tests/unittests/NetworkDetectorTests.cpp index b8d71bc94..385412943 100644 --- a/tests/unittests/NetworkDetectorTests.cpp +++ b/tests/unittests/NetworkDetectorTests.cpp @@ -2,7 +2,7 @@ #include "common/Common.hpp" -#ifdef _WIN32 +#if defined(_WIN32) && defined(HAVE_MAT_NETDETECT) #include "pal/desktop/NetworkDetector.hpp" using namespace MAT; From fa092818ac2a2a65fb65ddf2ca666b91afe79e6c Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 21 Sep 2026 18:09:40 -0500 Subject: [PATCH 19/23] Fix network shutdown and leak regression gaps Serialize WinRT status callbacks through the listener queue so Stop joins all event delivery, and replace timeout-based detachment with an explicit ready/failed startup state. Honor IConnectionCost2 background restrictions, schedule weekly analysis, and compare all leak metrics with the reviewed cross-platform baseline using non-blocking warnings. Files changed: - .github/memory-leak-baseline.csv - .github/scripts/run-drmemory.ps1 - .github/workflows/memory-leak-analysis.yml - lib/pal/desktop/NetworkDetector.cpp - lib/pal/desktop/NetworkDetector.hpp - tests/unittests/NetworkDetectorTests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/memory-leak-baseline.csv | 7 + .github/scripts/run-drmemory.ps1 | 50 +++++- .github/workflows/memory-leak-analysis.yml | 10 ++ lib/pal/desktop/NetworkDetector.cpp | 199 +++++++++++++-------- lib/pal/desktop/NetworkDetector.hpp | 24 ++- tests/unittests/NetworkDetectorTests.cpp | 42 +++-- 6 files changed, 238 insertions(+), 94 deletions(-) create mode 100644 .github/memory-leak-baseline.csv diff --git a/.github/memory-leak-baseline.csv b/.github/memory-leak-baseline.csv new file mode 100644 index 000000000..8bad9a45c --- /dev/null +++ b/.github/memory-leak-baseline.csv @@ -0,0 +1,7 @@ +Platform,Scenario,UniqueLeaks,TotalLeaks,LeakBytes,UniquePossibleLeaks,TotalPossibleLeaks,PossibleLeakBytes,UniqueReachable,TotalReachable,ReachableBytes +Windows,unit-tests,10,113,4256,14,15,7206,467,644,227717 +Windows,functional-tests,7,382,14072,7,3994,1038248411,1722,2823,725167 +Windows,sample-cpp-mini,0,0,0,0,0,0,973,1795,411008 +Linux,unit-tests,10,127,3730,5,7,3452,8,8,77045 +Linux,functional-tests,3,156,4694,1,1,4104,360,643,279619 +Linux,sample-cpp-mini,1,2,32,0,0,0,15,23,81130 diff --git a/.github/scripts/run-drmemory.ps1 b/.github/scripts/run-drmemory.ps1 index a56c7eb98..a4bcd1fcf 100644 --- a/.github/scripts/run-drmemory.ps1 +++ b/.github/scripts/run-drmemory.ps1 @@ -16,7 +16,9 @@ param( [ValidateNotNullOrEmpty()] [string]$TargetPath, - [string[]]$TargetArguments = @() + [string[]]$TargetArguments = @(), + + [string]$BaselinePath ) Set-StrictMode -Version Latest @@ -100,10 +102,50 @@ else { } $summaries | Export-Csv -LiteralPath $summaryPath -NoTypeInformation +$baselineStatus = "Not compared" +if ($BaselinePath) { + $resolvedBaselinePath = (Resolve-Path -LiteralPath $BaselinePath).Path + $baselineRows = @(Import-Csv -LiteralPath $resolvedBaselinePath | Where-Object { + $_.Platform -eq $summary.Platform -and $_.Scenario -eq $summary.Scenario + }) + if ($baselineRows.Count -ne 1) { + throw "Expected one baseline for $($summary.Platform)/$Scenario, found $($baselineRows.Count)." + } + + $regressions = @() + foreach ($metric in @( + "UniqueLeaks", + "TotalLeaks", + "LeakBytes", + "UniquePossibleLeaks", + "TotalPossibleLeaks", + "PossibleLeakBytes", + "UniqueReachable", + "TotalReachable", + "ReachableBytes" + )) { + $currentValue = [int64]$summary.$metric + $baselineValue = [int64]$baselineRows[0].$metric + if ($currentValue -gt $baselineValue) { + $regressions += "$metric increased from $baselineValue to $currentValue" + } + } + + if ($regressions.Count -eq 0) { + $baselineStatus = "At or below baseline" + } + else { + $baselineStatus = "$($regressions.Count) increase(s)" + foreach ($regression in $regressions) { + Write-Host "::warning title=Dr. Memory regression ($($summary.Platform)/$Scenario)::$regression" + } + } +} + $markdown = @" -| Scenario | Unique leaks | Total leaks | Leak bytes | Unique possible | Possible bytes | Unique reachable | Reachable bytes | -|---|---:|---:|---:|---:|---:|---:|---:| -| $Scenario | $($leaks.Unique) | $($leaks.Total) | $($leaks.Bytes) | $($possibleLeaks.Unique) | $($possibleLeaks.Bytes) | $($reachable.Unique) | $($reachable.Bytes) | +| Scenario | Unique leaks | Total leaks | Leak bytes | Unique possible | Possible bytes | Unique reachable | Reachable bytes | Baseline | +|---|---:|---:|---:|---:|---:|---:|---:|---| +| $Scenario | $($leaks.Unique) | $($leaks.Total) | $($leaks.Bytes) | $($possibleLeaks.Unique) | $($possibleLeaks.Bytes) | $($reachable.Unique) | $($reachable.Bytes) | $baselineStatus | "@ Write-Host $markdown if ($env:GITHUB_STEP_SUMMARY) { diff --git a/.github/workflows/memory-leak-analysis.yml b/.github/workflows/memory-leak-analysis.yml index a1f75e167..682316aa0 100644 --- a/.github/workflows/memory-leak-analysis.yml +++ b/.github/workflows/memory-leak-analysis.yml @@ -2,11 +2,14 @@ name: Memory leak analysis on: workflow_dispatch: + schedule: + - cron: 0 8 * * 1 push: branches: - main paths: - .github/scripts/run-drmemory.ps1 + - .github/memory-leak-baseline.csv - .github/workflows/memory-leak-analysis.yml - CMakeLists.txt - CMakePresets.json @@ -25,6 +28,7 @@ on: - main paths: - .github/scripts/run-drmemory.ps1 + - .github/memory-leak-baseline.csv - .github/workflows/memory-leak-analysis.yml permissions: @@ -88,6 +92,7 @@ jobs: -LogDirectory drmemory-results -Scenario unit-tests -TargetPath Solutions/out/Debug/x64/UnitTests/UnitTests.exe + -BaselinePath .github/memory-leak-baseline.csv -TargetArguments "--gtest_filter=-OfflineStorageTests_SQLite.StoreThousandEventsTakesLessThanASecond" - name: Analyze functional tests @@ -98,6 +103,7 @@ jobs: -LogDirectory drmemory-results -Scenario functional-tests -TargetPath Solutions/out/Debug/x64/FuncTests/FuncTests.exe + -BaselinePath .github/memory-leak-baseline.csv -TargetArguments "--gtest_filter=-BasicFuncTests.killSwitchWorks" - name: Analyze basic sample @@ -107,6 +113,7 @@ jobs: -DrMemoryPath "$env:RUNNER_TEMP/DrMemory-Windows-$env:DRMEMORY_VERSION/bin64/drmemory.exe" -LogDirectory drmemory-results -Scenario sample-cpp-mini + -BaselinePath .github/memory-leak-baseline.csv -TargetPath Solutions/out/Debug/x64/SampleCppMini/SampleCppMini.exe - name: Verify Network List Manager is not loaded @@ -186,6 +193,7 @@ jobs: -DrMemoryPath "$env:RUNNER_TEMP/DrMemory-Linux-$env:DRMEMORY_VERSION/bin64/drmemory" -LogDirectory drmemory-results -Scenario unit-tests + -BaselinePath .github/memory-leak-baseline.csv -TargetPath out/tests/unittests/UnitTests - name: Analyze functional tests @@ -196,6 +204,7 @@ jobs: -LogDirectory drmemory-results -Scenario functional-tests -TargetPath out/tests/functests/FuncTests + -BaselinePath .github/memory-leak-baseline.csv -TargetArguments "--gtest_filter=-BasicFuncTests.killSwitchWorks" - name: Analyze basic sample @@ -205,6 +214,7 @@ jobs: -DrMemoryPath "$env:RUNNER_TEMP/DrMemory-Linux-$env:DRMEMORY_VERSION/bin64/drmemory" -LogDirectory drmemory-results -Scenario sample-cpp-mini + -BaselinePath .github/memory-leak-baseline.csv -TargetPath out/sample-cpp-mini/SampleCppMini - name: Upload Linux reports diff --git a/lib/pal/desktop/NetworkDetector.cpp b/lib/pal/desktop/NetworkDetector.cpp index efd888352..ebc0ae8f4 100644 --- a/lib/pal/desktop/NetworkDetector.cpp +++ b/lib/pal/desktop/NetworkDetector.cpp @@ -15,27 +15,37 @@ #include "pal/PAL.hpp" #define NETDETECTOR_STOP WM_USER+1 -#define NETDETECTOR_START_TIMEOUT_MS 1000 +#define NETDETECTOR_REFRESH WM_USER+2 namespace MAT_NS_BEGIN { namespace Windows { struct NetworkDetector::CallbackState { - std::atomic acceptCallbacks{ true }; + std::atomic listenerThreadId{0}; + + bool QueueRefresh() const + { + const auto threadId = listenerThreadId.load(std::memory_order_acquire); + return threadId != 0 && + PostThreadMessage(threadId, NETDETECTOR_REFRESH, 0, NULL) != FALSE; + } }; NetworkCost MapNetworkCost( NetworkCostType costType, boolean roaming, boolean overDataLimit, - boolean approachingDataLimit) + boolean approachingDataLimit, + boolean backgroundDataUsageRestricted) { - if (roaming || overDataLimit || approachingDataLimit) { + if (roaming || overDataLimit || approachingDataLimit || backgroundDataUsageRestricted) + { return NetworkCost_Roaming; } - switch (costType) { + switch (costType) + { case NetworkCostType_Unrestricted: return NetworkCost_Unmetered; case NetworkCostType_Fixed: @@ -72,15 +82,29 @@ namespace MAT_NS_BEGIN boolean roaming = false; boolean overDataLimit = false; boolean approachingDataLimit = false; + boolean backgroundDataUsageRestricted = false; NetworkCostType costType = NetworkCostType_Unknown; if (FAILED(connectionCost->get_Roaming(&roaming)) || FAILED(connectionCost->get_OverDataLimit(&overDataLimit)) || FAILED(connectionCost->get_ApproachingDataLimit(&approachingDataLimit)) || - FAILED(connectionCost->get_NetworkCostType(&costType))) { + FAILED(connectionCost->get_NetworkCostType(&costType))) + { + return result; + } + + ComPtr connectionCost2; + if (SUCCEEDED(connectionCost.As(&connectionCost2)) && + FAILED(connectionCost2->get_BackgroundDataUsageRestricted(&backgroundDataUsageRestricted))) + { return result; } - return MapNetworkCost(costType, roaming, overDataLimit, approachingDataLimit); + return MapNetworkCost( + costType, + roaming, + overDataLimit, + approachingDataLimit, + backgroundDataUsageRestricted); } /// @@ -132,6 +156,16 @@ namespace MAT_NS_BEGIN return RefreshNetworkCost(networkInfoStats.Get(), *m_currentNetworkCost); } + bool NetworkDetector::QueueNetworkCostRefresh() + { + std::shared_ptr callbackState; + { + std::lock_guard lock(m_lock); + callbackState = networkStatusCallbackState; + } + return callbackState != nullptr && callbackState->QueueRefresh(); + } + /// /// Get activation factory and look-up network info statistics /// @@ -149,43 +183,54 @@ namespace MAT_NS_BEGIN bool NetworkDetector::RegisterAndListen() noexcept { - networkStatusCallbackState = std::make_shared(); + MSG msg; + PeekMessage(&msg, nullptr, WM_USER, WM_USER, PM_NOREMOVE); + const auto callbackState = networkStatusCallbackState; - const auto currentNetworkCost = m_currentNetworkCost; - const auto networkInformation = networkInfoStats; + callbackState->listenerThreadId.store(GetCurrentThreadId(), std::memory_order_release); networkStatusChangedHandler = Callback( - [callbackState, currentNetworkCost, networkInformation](IInspectable*) -> HRESULT { - if (callbackState->acceptCallbacks.load(std::memory_order_acquire)) { - RefreshNetworkCost(networkInformation.Get(), *currentNetworkCost); - } + [callbackState](IInspectable*) -> HRESULT + { + callbackState->QueueRefresh(); return S_OK; }); - if (networkStatusChangedHandler == nullptr) { + if (networkStatusChangedHandler == nullptr) + { LOG_ERROR("Unable to create network status handler."); - networkStatusCallbackState->acceptCallbacks.store(false, std::memory_order_release); - networkStatusCallbackState.reset(); + callbackState->listenerThreadId.store(0, std::memory_order_release); return false; } HRESULT hr = networkInfoStats->add_NetworkStatusChanged( networkStatusChangedHandler.Get(), &networkStatusChangedToken); - if (FAILED(hr)) { + if (FAILED(hr)) + { LOG_ERROR("Unable to subscribe to network status changes."); - networkStatusCallbackState->acceptCallbacks.store(false, std::memory_order_release); - networkStatusCallbackState.reset(); + callbackState->listenerThreadId.store(0, std::memory_order_release); networkStatusChangedHandler.Reset(); return false; } - MSG msg; - PeekMessage(&msg, nullptr, WM_USER, WM_USER, PM_NOREMOVE); - cv.notify_all(); + { + std::lock_guard lock(m_lock); + if (stopRequested) + { + startupState = StartupState::Failed; + cv.notify_all(); + return false; + } + startupState = StartupState::Ready; + cv.notify_all(); + } while (GetMessage(&msg, NULL, 0, 0) > 0) { switch (msg.message) { + case NETDETECTOR_REFRESH: + GetCurrentNetworkCost(); + break; case NETDETECTOR_STOP: PostQuitMessage(0); break; @@ -199,22 +244,21 @@ namespace MAT_NS_BEGIN } /// - /// + /// /// void NetworkDetector::Reset() { if (networkStatusCallbackState != nullptr) { - networkStatusCallbackState->acceptCallbacks.store(false, std::memory_order_release); + networkStatusCallbackState->listenerThreadId.store(0, std::memory_order_release); } - networkStatusChangedHandler.Reset(); - networkStatusCallbackState.reset(); if (networkStatusChangedToken.value != 0 && networkInfoStats != nullptr) { const auto token = networkStatusChangedToken; networkStatusChangedToken.value = 0; networkInfoStats->remove_NetworkStatusChanged(token); } + networkStatusChangedHandler.Reset(); networkInfoStats.Reset(); } @@ -271,18 +315,34 @@ namespace MAT_NS_BEGIN bool NetworkDetector::Start() { { - std::lock_guard lk(m_lock); - if (isRunning) + std::unique_lock lock(m_lock); + if (startupState == StartupState::Starting) + { + cv.wait(lock, [this]() + { return startupState != StartupState::Starting; }); + } + if (startupState == StartupState::Ready) { LOG_TRACE("NetworkDetector tid=%p is already running", m_listener_tid); return true; } + + lock.unlock(); + if (netDetectThread.joinable()) + { + netDetectThread.join(); + } + lock.lock(); + + startupState = StartupState::Starting; + stopRequested = false; + networkStatusCallbackState = std::make_shared(); isRunning = true; } // Start a new thread. Notify waiters on exit. netDetectThread = std::thread([this]() - { + { { std::lock_guard lk(m_lock); m_listener_tid = GetCurrentThreadId(); @@ -293,40 +353,37 @@ namespace MAT_NS_BEGIN std::lock_guard lk(m_lock); m_listener_tid = 0; isRunning = false; + if (startupState == StartupState::Starting) + { + startupState = StartupState::Failed; + } + else if (startupState == StartupState::Ready) + { + startupState = StartupState::Stopped; + } cv.notify_all(); - } - }); + } }); - if (netDetectThread.joinable()) { LOG_TRACE("NetworkDetector is starting..."); + bool started; { std::unique_lock lock(m_lock); - // Wait for up to NETDETECTOR_START_TIMEOUT_MS ms until: - // - the listener is subscribed; OR - // - Windows Runtime network information is unavailable - int retry = 1; - constexpr int max_retries = 2; - while (isRunning && cv.wait_for(lock, std::chrono::milliseconds(NETDETECTOR_START_TIMEOUT_MS)) - == std::cv_status::timeout && (retry < max_retries)) - { - LOG_TRACE("NetworkDetector starting up... [%u]", retry); - retry++; - } + cv.wait(lock, [this]() + { return startupState != StartupState::Starting; }); + started = startupState == StartupState::Ready; LOG_TRACE( "NetworkDetector tid=%p running=%u", m_listener_tid, - isRunning.load(std::memory_order_relaxed)); + started); } - } - else - { - std::lock_guard lk(m_lock); - LOG_WARN("NetworkDetector thread can't be started!"); - isRunning = false; - } - return isRunning.load(std::memory_order_relaxed); + if (!started && netDetectThread.joinable()) + { + netDetectThread.join(); + } + return started; + } }; /// @@ -336,31 +393,27 @@ namespace MAT_NS_BEGIN { if (netDetectThread.joinable()) { - std::unique_lock lk(m_lock); - try { - if (!isRunning || m_listener_tid == 0 || - !PostThreadMessage(m_listener_tid, NETDETECTOR_STOP, 0, NULL)) + { + std::lock_guard lock(m_lock); + stopRequested = true; + if (networkStatusCallbackState != nullptr) { - // Without detaching, we risk throwing an exception in the destructor. - // There is a chance that our code has finished, but the thread - // hasn't fully terminated, or the thread has already exited and - // isRunning is false. Alternatively, we may have never gotten - // a thread_id. - netDetectThread.detach(); - LOG_WARN("NetworkDetector thread unable to be shut down."); + networkStatusCallbackState->listenerThreadId.store(0, std::memory_order_release); } - else + if (startupState == StartupState::Ready && + !PostThreadMessage(m_listener_tid, NETDETECTOR_STOP, 0, NULL)) { - lk.unlock(); - netDetectThread.join(); - LOG_TRACE("NetworkDetector tid=%p has stopped.", m_listener_tid); + LOG_WARN("NetworkDetector stop message could not be posted."); } } - catch (std::system_error &ex) - { - UNREFERENCED_PARAMETER(ex); - LOG_WARN("NetworkDetector tid=%p is already stopped.", m_listener_tid); - } + + netDetectThread.join(); + + std::lock_guard lock(m_lock); + startupState = StartupState::Stopped; + stopRequested = false; + networkStatusCallbackState.reset(); + LOG_TRACE("NetworkDetector tid=%p has stopped.", m_listener_tid); } }; diff --git a/lib/pal/desktop/NetworkDetector.hpp b/lib/pal/desktop/NetworkDetector.hpp index c2fad0610..2e65748c1 100644 --- a/lib/pal/desktop/NetworkDetector.hpp +++ b/lib/pal/desktop/NetworkDetector.hpp @@ -41,13 +41,20 @@ namespace MAT_NS_BEGIN NetworkCostType costType, boolean roaming, boolean overDataLimit, - boolean approachingDataLimit); - - class NetworkDetector { - - private: + boolean approachingDataLimit, + boolean backgroundDataUsageRestricted); + class NetworkDetector + { + private: struct CallbackState; + enum class StartupState + { + Stopped, + Starting, + Ready, + Failed + }; /// /// Current network info stats @@ -68,6 +75,8 @@ namespace MAT_NS_BEGIN std::condition_variable cv; std::atomic isRunning{ false }; std::thread netDetectThread; + StartupState startupState = StartupState::Stopped; + bool stopRequested = false; /// /// @@ -131,8 +140,11 @@ namespace MAT_NS_BEGIN /// NetworkCost GetNetworkCost(); + /// + /// Queue the same refresh performed by a WinRT network status callback. + /// + bool QueueNetworkCostRefresh(); }; - } } MAT_NS_END diff --git a/tests/unittests/NetworkDetectorTests.cpp b/tests/unittests/NetworkDetectorTests.cpp index 385412943..5ba48394b 100644 --- a/tests/unittests/NetworkDetectorTests.cpp +++ b/tests/unittests/NetworkDetectorTests.cpp @@ -10,17 +10,18 @@ using namespace testing; TEST(NetworkDetectorTests, MapsWinRTNetworkCosts) { - EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unrestricted, false, false, false), NetworkCost_Unmetered); - EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Fixed, false, false, false), NetworkCost_Metered); - EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Variable, false, false, false), NetworkCost_Metered); - EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unknown, false, false, false), NetworkCost_Unknown); + EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unrestricted, false, false, false, false), NetworkCost_Unmetered); + EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Fixed, false, false, false, false), NetworkCost_Metered); + EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Variable, false, false, false, false), NetworkCost_Metered); + EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unknown, false, false, false, false), NetworkCost_Unknown); } TEST(NetworkDetectorTests, MapsRestrictiveWinRTNetworkStates) { - EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unrestricted, true, false, false), NetworkCost_Roaming); - EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unrestricted, false, true, false), NetworkCost_Roaming); - EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unrestricted, false, false, true), NetworkCost_Roaming); + EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unrestricted, true, false, false, false), NetworkCost_Roaming); + EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unrestricted, false, true, false, false), NetworkCost_Roaming); + EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unrestricted, false, false, true, false), NetworkCost_Roaming); + EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unrestricted, false, false, false, true), NetworkCost_Roaming); } TEST(NetworkDetectorTests, StartsReadsCostAndStopsWithoutNetworkListManager) @@ -34,14 +35,33 @@ TEST(NetworkDetectorTests, StartsReadsCostAndStopsWithoutNetworkListManager) const auto cost = detector.GetCurrentNetworkCost(); EXPECT_THAT(cost, AnyOf( - Eq(NetworkCost_Unknown), - Eq(NetworkCost_Unmetered), - Eq(NetworkCost_Metered), - Eq(NetworkCost_Roaming))); + Eq(NetworkCost_Unknown), + Eq(NetworkCost_Unmetered), + Eq(NetworkCost_Metered), + Eq(NetworkCost_Roaming))); EXPECT_EQ(detector.GetNetworkCost(), cost); detector.Stop(); EXPECT_FALSE(detector.isUp()); + EXPECT_FALSE(detector.QueueNetworkCostRefresh()); EXPECT_EQ(GetModuleHandleW(L"netprofm.dll"), nullptr); } + +TEST(NetworkDetectorTests, QueuedNetworkCallbackRaceDoesNotOutliveStop) +{ + MATW::NetworkDetector detector; + ASSERT_TRUE(detector.Start()); + + std::atomic keepQueuing{true}; + std::thread callbackThread([&]() + { + while (keepQueuing.load(std::memory_order_acquire)) { + detector.QueueNetworkCostRefresh(); + } }); + + detector.Stop(); + EXPECT_FALSE(detector.QueueNetworkCostRefresh()); + keepQueuing.store(false, std::memory_order_release); + callbackThread.join(); +} #endif From 3dc04b1ea4c022b9af886463ea7b94fb4f4261ce Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 21 Sep 2026 18:20:59 -0500 Subject: [PATCH 20/23] Prevent network shutdown queue starvation Use a kernel stop event with MsgWaitForMultipleObjects so callback refresh traffic cannot prevent listener termination or make Stop hang after a failed PostThreadMessage. Verified at: - lib/pal/desktop/NetworkDetector.cpp - lib/pal/desktop/NetworkDetector.hpp Files changed: - lib/pal/desktop/NetworkDetector.cpp - lib/pal/desktop/NetworkDetector.hpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/pal/desktop/NetworkDetector.cpp | 59 +++++++++++++++++++++++------ lib/pal/desktop/NetworkDetector.hpp | 9 +++-- 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/lib/pal/desktop/NetworkDetector.cpp b/lib/pal/desktop/NetworkDetector.cpp index ebc0ae8f4..0e9e2dce9 100644 --- a/lib/pal/desktop/NetworkDetector.cpp +++ b/lib/pal/desktop/NetworkDetector.cpp @@ -14,8 +14,7 @@ #include "DebugEvents.hpp" #include "pal/PAL.hpp" -#define NETDETECTOR_STOP WM_USER+1 -#define NETDETECTOR_REFRESH WM_USER+2 +#define NETDETECTOR_REFRESH WM_USER+1 namespace MAT_NS_BEGIN { @@ -224,21 +223,41 @@ namespace MAT_NS_BEGIN cv.notify_all(); } - while (GetMessage(&msg, NULL, 0, 0) > 0) + while (true) { + const DWORD waitResult = MsgWaitForMultipleObjects( + 1, + &stopEvent, + FALSE, + INFINITE, + QS_ALLINPUT); + if (waitResult == WAIT_OBJECT_0) + { + break; + } + if (waitResult == WAIT_FAILED) + { + LOG_ERROR("Unable to wait for network detector events."); + return false; + } + if (!PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) + { + continue; + } + if (msg.message == WM_QUIT) + { + break; + } + switch (msg.message) { case NETDETECTOR_REFRESH: GetCurrentNetworkCost(); break; - case NETDETECTOR_STOP: - PostQuitMessage(0); - break; default: - break; + TranslateMessage(&msg); + DispatchMessage(&msg); } - TranslateMessage(&msg); - DispatchMessage(&msg); } return true; } @@ -337,6 +356,14 @@ namespace MAT_NS_BEGIN startupState = StartupState::Starting; stopRequested = false; networkStatusCallbackState = std::make_shared(); + stopEvent = CreateEventW(nullptr, TRUE, FALSE, nullptr); + if (stopEvent == nullptr) + { + LOG_ERROR("Unable to create the network detector stop event."); + startupState = StartupState::Failed; + networkStatusCallbackState.reset(); + return false; + } isRunning = true; } @@ -382,6 +409,13 @@ namespace MAT_NS_BEGIN { netDetectThread.join(); } + if (!started) + { + std::lock_guard lock(m_lock); + CloseHandle(stopEvent); + stopEvent = nullptr; + networkStatusCallbackState.reset(); + } return started; } }; @@ -400,16 +434,17 @@ namespace MAT_NS_BEGIN { networkStatusCallbackState->listenerThreadId.store(0, std::memory_order_release); } - if (startupState == StartupState::Ready && - !PostThreadMessage(m_listener_tid, NETDETECTOR_STOP, 0, NULL)) + if (!SetEvent(stopEvent)) { - LOG_WARN("NetworkDetector stop message could not be posted."); + LOG_ERROR("Unable to signal the network detector stop event."); } } netDetectThread.join(); std::lock_guard lock(m_lock); + CloseHandle(stopEvent); + stopEvent = nullptr; startupState = StartupState::Stopped; stopRequested = false; networkStatusCallbackState.reset(); diff --git a/lib/pal/desktop/NetworkDetector.hpp b/lib/pal/desktop/NetworkDetector.hpp index 2e65748c1..f87d423dc 100644 --- a/lib/pal/desktop/NetworkDetector.hpp +++ b/lib/pal/desktop/NetworkDetector.hpp @@ -72,14 +72,15 @@ namespace MAT_NS_BEGIN bool GetNetworkInfoStats(); std::mutex m_lock; - std::condition_variable cv; - std::atomic isRunning{ false }; - std::thread netDetectThread; + std::condition_variable cv; + std::atomic isRunning{false}; + std::thread netDetectThread; StartupState startupState = StartupState::Stopped; bool stopRequested = false; + HANDLE stopEvent = nullptr; /// - /// + /// /// void run(); From a30f5fd0582fc3228015cd1c1da7e04901c3179e Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 21 Sep 2026 18:34:38 -0500 Subject: [PATCH 21/23] Keep queued network refreshes observable Use MsgWaitForMultipleObjectsEx with MWMO_INPUTAVAILABLE so refresh messages remain visible after earlier queue inspection while the kernel stop event retains shutdown priority. Verified at lib/pal/desktop/NetworkDetector.cpp. Files changed: - lib/pal/desktop/NetworkDetector.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/pal/desktop/NetworkDetector.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/pal/desktop/NetworkDetector.cpp b/lib/pal/desktop/NetworkDetector.cpp index 0e9e2dce9..b53d57ae7 100644 --- a/lib/pal/desktop/NetworkDetector.cpp +++ b/lib/pal/desktop/NetworkDetector.cpp @@ -225,12 +225,12 @@ namespace MAT_NS_BEGIN while (true) { - const DWORD waitResult = MsgWaitForMultipleObjects( + const DWORD waitResult = MsgWaitForMultipleObjectsEx( 1, &stopEvent, - FALSE, INFINITE, - QS_ALLINPUT); + QS_ALLINPUT, + MWMO_INPUTAVAILABLE); if (waitResult == WAIT_OBJECT_0) { break; From 1b3fc58dae1eed48f9224313b06cc5937a60da90 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 21 Sep 2026 18:49:58 -0500 Subject: [PATCH 22/23] Serialize network detector lifecycle changes Protect all Start and Stop access to the worker thread with a dedicated lifecycle mutex so shutdown cannot miss an unpublished thread. Add a concurrent startup/shutdown regression test. Show total possible leaks and total reachable allocations in the job summary so every baseline-checked metric is directly interpretable. Files changed: - .github/scripts/run-drmemory.ps1 - lib/pal/desktop/NetworkDetector.cpp - lib/pal/desktop/NetworkDetector.hpp - tests/unittests/NetworkDetectorTests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/scripts/run-drmemory.ps1 | 6 +++--- lib/pal/desktop/NetworkDetector.cpp | 2 ++ lib/pal/desktop/NetworkDetector.hpp | 12 ++++++------ tests/unittests/NetworkDetectorTests.cpp | 22 ++++++++++++++++++++++ 4 files changed, 33 insertions(+), 9 deletions(-) diff --git a/.github/scripts/run-drmemory.ps1 b/.github/scripts/run-drmemory.ps1 index a4bcd1fcf..67f7a87dd 100644 --- a/.github/scripts/run-drmemory.ps1 +++ b/.github/scripts/run-drmemory.ps1 @@ -143,9 +143,9 @@ if ($BaselinePath) { } $markdown = @" -| Scenario | Unique leaks | Total leaks | Leak bytes | Unique possible | Possible bytes | Unique reachable | Reachable bytes | Baseline | -|---|---:|---:|---:|---:|---:|---:|---:|---| -| $Scenario | $($leaks.Unique) | $($leaks.Total) | $($leaks.Bytes) | $($possibleLeaks.Unique) | $($possibleLeaks.Bytes) | $($reachable.Unique) | $($reachable.Bytes) | $baselineStatus | +| Scenario | Unique leaks | Total leaks | Leak bytes | Unique possible | Total possible | Possible bytes | Unique reachable | Total reachable | Reachable bytes | Baseline | +|---|---:|---:|---:|---:|---:|---:|---:|---:|---:|---| +| $Scenario | $($leaks.Unique) | $($leaks.Total) | $($leaks.Bytes) | $($possibleLeaks.Unique) | $($possibleLeaks.Total) | $($possibleLeaks.Bytes) | $($reachable.Unique) | $($reachable.Total) | $($reachable.Bytes) | $baselineStatus | "@ Write-Host $markdown if ($env:GITHUB_STEP_SUMMARY) { diff --git a/lib/pal/desktop/NetworkDetector.cpp b/lib/pal/desktop/NetworkDetector.cpp index b53d57ae7..901479ca1 100644 --- a/lib/pal/desktop/NetworkDetector.cpp +++ b/lib/pal/desktop/NetworkDetector.cpp @@ -333,6 +333,7 @@ namespace MAT_NS_BEGIN /// true - if start is successful, false - otherwise bool NetworkDetector::Start() { + std::lock_guard lifecycleLock(m_lifecycleLock); { std::unique_lock lock(m_lock); if (startupState == StartupState::Starting) @@ -425,6 +426,7 @@ namespace MAT_NS_BEGIN /// void NetworkDetector::Stop() { + std::lock_guard lifecycleLock(m_lifecycleLock); if (netDetectThread.joinable()) { { diff --git a/lib/pal/desktop/NetworkDetector.hpp b/lib/pal/desktop/NetworkDetector.hpp index f87d423dc..930686278 100644 --- a/lib/pal/desktop/NetworkDetector.hpp +++ b/lib/pal/desktop/NetworkDetector.hpp @@ -59,11 +59,10 @@ namespace MAT_NS_BEGIN /// /// Current network info stats /// - ComPtr networkInfoStats; + ComPtr networkInfoStats; ComPtr networkStatusChangedHandler; - EventRegistrationToken networkStatusChangedToken{}; - std::shared_ptr networkStatusCallbackState; - + EventRegistrationToken networkStatusChangedToken{}; + std::shared_ptr networkStatusCallbackState; /// /// Get instance of network info stats @@ -71,7 +70,8 @@ namespace MAT_NS_BEGIN /// bool GetNetworkInfoStats(); - std::mutex m_lock; + std::mutex m_lifecycleLock; + std::mutex m_lock; std::condition_variable cv; std::atomic isRunning{false}; std::thread netDetectThread; @@ -84,7 +84,7 @@ namespace MAT_NS_BEGIN /// void run(); - DWORD m_listener_tid = 0; + DWORD m_listener_tid = 0; /// /// Register and listen to network state notifications diff --git a/tests/unittests/NetworkDetectorTests.cpp b/tests/unittests/NetworkDetectorTests.cpp index 5ba48394b..359f4319c 100644 --- a/tests/unittests/NetworkDetectorTests.cpp +++ b/tests/unittests/NetworkDetectorTests.cpp @@ -64,4 +64,26 @@ TEST(NetworkDetectorTests, QueuedNetworkCallbackRaceDoesNotOutliveStop) keepQueuing.store(false, std::memory_order_release); callbackThread.join(); } + +TEST(NetworkDetectorTests, ConcurrentStopWaitsForStartupPublication) +{ + for (int iteration = 0; iteration < 20; ++iteration) + { + MATW::NetworkDetector detector; + std::atomic startReturned{false}; + std::thread startThread([&]() + { + detector.Start(); + startReturned.store(true, std::memory_order_release); }); + + while (!detector.isUp() && !startReturned.load(std::memory_order_acquire)) + { + std::this_thread::yield(); + } + + detector.Stop(); + startThread.join(); + EXPECT_FALSE(detector.isUp()); + } +} #endif From d6bcef3ad26fc6ac0eba57b7a04cee738e08e58e Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 21 Sep 2026 19:04:04 -0500 Subject: [PATCH 23/23] Avoid reentrant network listener self-join Dispatch coalesced network debug events through an independently owned Windows worker callback. Stop disables future events and waits for external dispatches, but safely returns when called by the active event itself after joining the detector listener. Add a regression test whose EVT_NET_CHANGED listener synchronously stops the detector; it passes repeatedly without self-join or deadlock. Files changed: - lib/pal/desktop/NetworkDetector.cpp - lib/pal/desktop/NetworkDetector.hpp - tests/unittests/NetworkDetectorTests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/pal/desktop/NetworkDetector.cpp | 155 +++++++++++++++++++---- lib/pal/desktop/NetworkDetector.hpp | 2 + tests/unittests/NetworkDetectorTests.cpp | 50 ++++++++ 3 files changed, 182 insertions(+), 25 deletions(-) diff --git a/lib/pal/desktop/NetworkDetector.cpp b/lib/pal/desktop/NetworkDetector.cpp index 901479ca1..c6185a895 100644 --- a/lib/pal/desktop/NetworkDetector.cpp +++ b/lib/pal/desktop/NetworkDetector.cpp @@ -10,17 +10,21 @@ #include "NetworkDetector.hpp" -#include "ILogManager.hpp" #include "DebugEvents.hpp" +#include "ILogManager.hpp" #include "pal/PAL.hpp" -#define NETDETECTOR_REFRESH WM_USER+1 +#define NETDETECTOR_REFRESH WM_USER + 1 namespace MAT_NS_BEGIN { - namespace Windows { + namespace Windows + { - struct NetworkDetector::CallbackState { + static thread_local void* currentNetworkEventDispatch = nullptr; + + struct NetworkDetector::CallbackState + { std::atomic listenerThreadId{0}; bool QueueRefresh() const @@ -31,6 +35,88 @@ namespace MAT_NS_BEGIN } }; + struct NetworkDetector::EventDispatchState : std::enable_shared_from_this + { + bool Queue(NetworkCost cost) + { + std::lock_guard lock(mutex); + if (!acceptEvents) + { + return false; + } + + latestCost = cost; + eventPending = true; + if (workerScheduled) + { + return true; + } + + workerScheduled = true; + auto context = new (std::nothrow) std::shared_ptr(shared_from_this()); + if (context == nullptr || + !QueueUserWorkItem(DispatchPendingEvents, context, WT_EXECUTEDEFAULT)) + { + delete context; + workerScheduled = false; + return false; + } + return true; + } + + void StopAndWait() + { + std::unique_lock lock(mutex); + acceptEvents = false; + eventPending = false; + if (currentNetworkEventDispatch == this) + { + return; + } + cv.wait(lock, [this]() + { return !workerScheduled; }); + } + + private: + static DWORD CALLBACK DispatchPendingEvents(void* context) + { + std::shared_ptr state = + *static_cast*>(context); + delete static_cast*>(context); + currentNetworkEventDispatch = state.get(); + + while (true) + { + NetworkCost cost; + { + std::lock_guard lock(state->mutex); + if (!state->acceptEvents || !state->eventPending) + { + state->workerScheduled = false; + state->cv.notify_all(); + currentNetworkEventDispatch = nullptr; + return 0; + } + cost = state->latestCost; + state->eventPending = false; + } + + DebugEvent evt; + evt.type = DebugEventType::EVT_NET_CHANGED; + evt.param1 = cost; + evt.param2 = false; + ILogManager::DispatchEventBroadcast(evt); + } + } + + std::mutex mutex; + std::condition_variable cv; + NetworkCost latestCost = NetworkCost_Unknown; + bool acceptEvents = true; + bool eventPending = false; + bool workerScheduled = false; + }; + NetworkCost MapNetworkCost( NetworkCostType costType, boolean roaming, @@ -61,20 +147,23 @@ namespace MAT_NS_BEGIN NetworkCost result = NetworkCost_Unknown; LOG_TRACE("get network cost...\n"); - if (networkInfoStats == nullptr) { + if (networkInfoStats == nullptr) + { LOG_WARN("Windows network information is unavailable!"); return result; } ComPtr connectionProfile; HRESULT hr = networkInfoStats->GetInternetConnectionProfile(&connectionProfile); - if (FAILED(hr) || connectionProfile == nullptr) { + if (FAILED(hr) || connectionProfile == nullptr) + { return result; } ComPtr connectionCost; hr = connectionProfile->GetConnectionCost(&connectionCost); - if (FAILED(hr) || connectionCost == nullptr) { + if (FAILED(hr) || connectionCost == nullptr) + { return result; } @@ -111,13 +200,14 @@ namespace MAT_NS_BEGIN /// This function provides an SEH handler for Windows Runtime failures. /// #pragma warning(push) -#pragma warning(disable: 6320) +#pragma warning(disable : 6320) static int RefreshNetworkCost( INetworkInformationStatics* networkInfoStats, std::atomic& currentNetworkCostState) { NetworkCost currentNetworkCost = NetworkCost_Unknown; - __try { + __try + { currentNetworkCost = QueryCurrentNetworkCost(networkInfoStats); } //****************************************************************************************************************************** @@ -135,24 +225,29 @@ namespace MAT_NS_BEGIN } currentNetworkCostState.store(currentNetworkCost, std::memory_order_relaxed); - - DebugEvent evt; - evt.type = DebugEventType::EVT_NET_CHANGED; - evt.param1 = currentNetworkCost; - evt.param2 = false; - ILogManager::DispatchEventBroadcast(evt); - return currentNetworkCost; } #pragma warning(pop) - NetworkCost NetworkDetector::GetNetworkCost() { + NetworkCost NetworkDetector::GetNetworkCost() + { return m_currentNetworkCost->load(std::memory_order_relaxed); } int NetworkDetector::GetCurrentNetworkCost() { - return RefreshNetworkCost(networkInfoStats.Get(), *m_currentNetworkCost); + const auto currentNetworkCost = + RefreshNetworkCost(networkInfoStats.Get(), *m_currentNetworkCost); + std::shared_ptr dispatchState; + { + std::lock_guard lock(m_lock); + dispatchState = eventDispatchState; + } + if (dispatchState != nullptr && !dispatchState->Queue(static_cast(currentNetworkCost))) + { + LOG_WARN("Unable to queue network status event."); + } + return currentNetworkCost; } bool NetworkDetector::QueueNetworkCostRefresh() @@ -222,6 +317,10 @@ namespace MAT_NS_BEGIN startupState = StartupState::Ready; cv.notify_all(); } + if (!eventDispatchState->Queue(GetNetworkCost())) + { + LOG_WARN("Unable to queue initial network status event."); + } while (true) { @@ -284,8 +383,8 @@ namespace MAT_NS_BEGIN /// /// Register for Windows Runtime events and block-wait in RegisterAndListen /// -#pragma warning( push ) -#pragma warning(disable:6320) +#pragma warning(push) +#pragma warning(disable : 6320) void NetworkDetector::run() { bool isRoInitialized = false; @@ -304,7 +403,7 @@ namespace MAT_NS_BEGIN isRoInitialized = true; if (GetNetworkInfoStats()) { - GetCurrentNetworkCost(); + RefreshNetworkCost(networkInfoStats.Get(), *m_currentNetworkCost); LOG_TRACE("start listening to events..."); RegisterAndListen(); } @@ -323,9 +422,8 @@ namespace MAT_NS_BEGIN { RoUninitialize(); } - } -#pragma warning( pop ) +#pragma warning(pop) /// /// Start network monitoring thread @@ -357,12 +455,14 @@ namespace MAT_NS_BEGIN startupState = StartupState::Starting; stopRequested = false; networkStatusCallbackState = std::make_shared(); + eventDispatchState = std::make_shared(); stopEvent = CreateEventW(nullptr, TRUE, FALSE, nullptr); if (stopEvent == nullptr) { LOG_ERROR("Unable to create the network detector stop event."); startupState = StartupState::Failed; networkStatusCallbackState.reset(); + eventDispatchState.reset(); return false; } isRunning = true; @@ -416,6 +516,8 @@ namespace MAT_NS_BEGIN CloseHandle(stopEvent); stopEvent = nullptr; networkStatusCallbackState.reset(); + eventDispatchState->StopAndWait(); + eventDispatchState.reset(); } return started; } @@ -443,6 +545,7 @@ namespace MAT_NS_BEGIN } netDetectThread.join(); + eventDispatchState->StopAndWait(); std::lock_guard lock(m_lock); CloseHandle(stopEvent); @@ -450,6 +553,7 @@ namespace MAT_NS_BEGIN startupState = StartupState::Stopped; stopRequested = false; networkStatusCallbackState.reset(); + eventDispatchState.reset(); LOG_TRACE("NetworkDetector tid=%p has stopped.", m_listener_tid); } }; @@ -465,8 +569,9 @@ namespace MAT_NS_BEGIN LOG_TRACE("NetworkDetector done tid=%p", m_listener_tid); } - } // ::Windows + } // ::Windows -} MAT_NS_END +} +MAT_NS_END #endif diff --git a/lib/pal/desktop/NetworkDetector.hpp b/lib/pal/desktop/NetworkDetector.hpp index 930686278..2b4080e04 100644 --- a/lib/pal/desktop/NetworkDetector.hpp +++ b/lib/pal/desktop/NetworkDetector.hpp @@ -48,6 +48,7 @@ namespace MAT_NS_BEGIN { private: struct CallbackState; + struct EventDispatchState; enum class StartupState { Stopped, @@ -63,6 +64,7 @@ namespace MAT_NS_BEGIN ComPtr networkStatusChangedHandler; EventRegistrationToken networkStatusChangedToken{}; std::shared_ptr networkStatusCallbackState; + std::shared_ptr eventDispatchState; /// /// Get instance of network info stats diff --git a/tests/unittests/NetworkDetectorTests.cpp b/tests/unittests/NetworkDetectorTests.cpp index 359f4319c..0103a4c11 100644 --- a/tests/unittests/NetworkDetectorTests.cpp +++ b/tests/unittests/NetworkDetectorTests.cpp @@ -3,11 +3,42 @@ #include "common/Common.hpp" #if defined(_WIN32) && defined(HAVE_MAT_NETDETECT) +#include "api/LogManagerFactory.hpp" #include "pal/desktop/NetworkDetector.hpp" +#include + using namespace MAT; using namespace testing; +class StopDetectorOnNetworkChange : public DebugEventListener +{ + public: + explicit StopDetectorOnNetworkChange(MATW::NetworkDetector& detector) : + detector(detector) + { + } + + void OnDebugEvent(DebugEvent& event) override + { + if (event.type == EVT_NET_CHANGED && !handled.exchange(true)) + { + detector.Stop(); + stopped.set_value(); + } + } + + std::future GetStoppedFuture() + { + return stopped.get_future(); + } + + private: + MATW::NetworkDetector& detector; + std::atomic handled{false}; + std::promise stopped; +}; + TEST(NetworkDetectorTests, MapsWinRTNetworkCosts) { EXPECT_EQ(MATW::MapNetworkCost(NetworkCostType_Unrestricted, false, false, false, false), NetworkCost_Unmetered); @@ -86,4 +117,23 @@ TEST(NetworkDetectorTests, ConcurrentStopWaitsForStartupPublication) EXPECT_FALSE(detector.isUp()); } } + +TEST(NetworkDetectorTests, NetworkChangeListenerCanStopDetector) +{ + ILogConfiguration configuration; + configuration[CFG_BOOL_ENABLE_NET_DETECT] = false; + ILogManager* logManager = LogManagerFactory::Create(configuration); + ASSERT_NE(logManager, nullptr); + MATW::NetworkDetector detector; + StopDetectorOnNetworkChange listener(detector); + auto stopped = listener.GetStoppedFuture(); + logManager->AddEventListener(EVT_NET_CHANGED, listener); + + ASSERT_TRUE(detector.Start()); + ASSERT_EQ(stopped.wait_for(std::chrono::seconds(5)), std::future_status::ready); + EXPECT_FALSE(detector.isUp()); + + logManager->RemoveEventListener(EVT_NET_CHANGED, listener); + EXPECT_EQ(LogManagerFactory::Destroy(logManager), STATUS_SUCCESS); +} #endif