Skip to content

Commit 1b3fc58

Browse files
bmehta001Copilot
andcommitted
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>
1 parent a30f5fd commit 1b3fc58

4 files changed

Lines changed: 33 additions & 9 deletions

File tree

‎.github/scripts/run-drmemory.ps1‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,9 @@ if ($BaselinePath) {
143143
}
144144

145145
$markdown = @"
146-
| Scenario | Unique leaks | Total leaks | Leak bytes | Unique possible | Possible bytes | Unique reachable | Reachable bytes | Baseline |
147-
|---|---:|---:|---:|---:|---:|---:|---:|---|
148-
| $Scenario | $($leaks.Unique) | $($leaks.Total) | $($leaks.Bytes) | $($possibleLeaks.Unique) | $($possibleLeaks.Bytes) | $($reachable.Unique) | $($reachable.Bytes) | $baselineStatus |
146+
| Scenario | Unique leaks | Total leaks | Leak bytes | Unique possible | Total possible | Possible bytes | Unique reachable | Total reachable | Reachable bytes | Baseline |
147+
|---|---:|---:|---:|---:|---:|---:|---:|---:|---:|---|
148+
| $Scenario | $($leaks.Unique) | $($leaks.Total) | $($leaks.Bytes) | $($possibleLeaks.Unique) | $($possibleLeaks.Total) | $($possibleLeaks.Bytes) | $($reachable.Unique) | $($reachable.Total) | $($reachable.Bytes) | $baselineStatus |
149149
"@
150150
Write-Host $markdown
151151
if ($env:GITHUB_STEP_SUMMARY) {

‎lib/pal/desktop/NetworkDetector.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,7 @@ namespace MAT_NS_BEGIN
333333
/// <returns>true - if start is successful, false - otherwise</returns>
334334
bool NetworkDetector::Start()
335335
{
336+
std::lock_guard<std::mutex> lifecycleLock(m_lifecycleLock);
336337
{
337338
std::unique_lock<std::mutex> lock(m_lock);
338339
if (startupState == StartupState::Starting)
@@ -425,6 +426,7 @@ namespace MAT_NS_BEGIN
425426
/// </summary>
426427
void NetworkDetector::Stop()
427428
{
429+
std::lock_guard<std::mutex> lifecycleLock(m_lifecycleLock);
428430
if (netDetectThread.joinable())
429431
{
430432
{

‎lib/pal/desktop/NetworkDetector.hpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,19 +59,19 @@ namespace MAT_NS_BEGIN
5959
/// <summary>
6060
/// Current network info stats
6161
/// </summary>
62-
ComPtr<INetworkInformationStatics> networkInfoStats;
62+
ComPtr<INetworkInformationStatics> networkInfoStats;
6363
ComPtr<INetworkStatusChangedEventHandler> networkStatusChangedHandler;
64-
EventRegistrationToken networkStatusChangedToken{};
65-
std::shared_ptr<CallbackState> networkStatusCallbackState;
66-
64+
EventRegistrationToken networkStatusChangedToken{};
65+
std::shared_ptr<CallbackState> networkStatusCallbackState;
6766

6867
/// <summary>
6968
/// Get instance of network info stats
7069
/// </summary>
7170
/// <returns></returns>
7271
bool GetNetworkInfoStats();
7372

74-
std::mutex m_lock;
73+
std::mutex m_lifecycleLock;
74+
std::mutex m_lock;
7575
std::condition_variable cv;
7676
std::atomic<bool> isRunning{false};
7777
std::thread netDetectThread;
@@ -84,7 +84,7 @@ namespace MAT_NS_BEGIN
8484
/// </summary>
8585
void run();
8686

87-
DWORD m_listener_tid = 0;
87+
DWORD m_listener_tid = 0;
8888

8989
/// <summary>
9090
/// Register and listen to network state notifications

‎tests/unittests/NetworkDetectorTests.cpp‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,26 @@ TEST(NetworkDetectorTests, QueuedNetworkCallbackRaceDoesNotOutliveStop)
6464
keepQueuing.store(false, std::memory_order_release);
6565
callbackThread.join();
6666
}
67+
68+
TEST(NetworkDetectorTests, ConcurrentStopWaitsForStartupPublication)
69+
{
70+
for (int iteration = 0; iteration < 20; ++iteration)
71+
{
72+
MATW::NetworkDetector detector;
73+
std::atomic<bool> startReturned{false};
74+
std::thread startThread([&]()
75+
{
76+
detector.Start();
77+
startReturned.store(true, std::memory_order_release); });
78+
79+
while (!detector.isUp() && !startReturned.load(std::memory_order_acquire))
80+
{
81+
std::this_thread::yield();
82+
}
83+
84+
detector.Stop();
85+
startThread.join();
86+
EXPECT_FALSE(detector.isUp());
87+
}
88+
}
6789
#endif

0 commit comments

Comments
 (0)