🐞 Bug: Detached thread in ApplyCallback may access invalidated pointers (lifecycle violation)

[Silver-Yang]

Related issue：

Issue 852： Core-dump after SandboxDestroyCallback on arm32 plateform with version 1.2.0

Description

In ApplyCallback, a detached worker thread is created to perform the async apply operation:

std::thread worker{ [token, workCompletionData, workflowData] {
    const ADUC_Result result{ ADUC::ExceptionUtils::CallResultMethodAndHandleExceptions(
        ADUC_Result_Failure, [&token, &workCompletionData, &workflowData]() -> ADUC_Result {
            return static_cast<LinuxPlatformLayer*>(token)->Apply(workflowData);
        }) };

    workCompletionData->WorkCompletionCallback(
        workCompletionData->WorkCompletionToken, result, true /* isAsync */);
} };

worker.detach();

However, this introduces a lifecycle issue:

• The thread is detached, meaning it runs independently of the caller.
• The lambda captures raw pointers (workCompletionData, workflowData) which are not owned by the thread.
• If the main thread (or workflow) exits before the worker thread completes, these pointers may become invalid.

This contradicts the comment in the code:

// Pointers passed to this method are guaranteed to be valid until WorkCompletionCallback is called.

There is currently no mechanism enforcing this guarantee when using a detached thread.

---

Impact

This can lead to:

• Use-after-free
• Undefined behavior
• Crash / core dump during async apply
• Non-deterministic failures depending on timing

This is especially likely when:

• The agent shuts down during an ongoing apply
• The workflow lifecycle ends early
• Errors or reboots occur before callback execution

---

Expected Behavior

One of the following should be guaranteed:

1. The lifetime of workflowData and workCompletionData is extended until the worker thread completes and calls WorkCompletionCallback, or
2. The worker thread must not outlive the owning context (i.e., should not be detached without lifecycle control)

---

Actual Behavior

• The worker thread is detached
• No ownership or lifetime management is enforced
• The thread may access invalid memory if the main thread exits early

---

Root Cause Analysis

• Use of std::thread::detach() without lifecycle synchronization
• Capturing raw pointers into an async execution context
• Lack of ownership model (no shared_ptr / ref-counting / cancellation)

---

Suggested Fixes

Possible approaches:

1. Avoid detach()

   • Use joinable threads or a managed thread pool

2. Introduce shared ownership

   • Use std::shared_ptr for workflowData and related structures

3. Agent-managed async execution

   • Let the agent manage thread lifecycle instead of handler

4. Shutdown / cancellation support

   • Ensure worker threads are stopped or joined during agent shutdown

---

Additional Notes

This issue appears to be a general concurrency/lifecycle design flaw and may affect other async handlers using similar patterns.

---

Environment

• Component: LinuxPlatformLayer / ApplyCallback
• Version: (please fill in)
• Reproducibility: Intermittent (timing-dependent)

---

Summary

Detached threads combined with raw pointer capture can lead to use-after-free and crashes. This is a lifecycle management bug rather than intended behavior.

[Silver-Yang]

A potential fix. Patch based on v1.2.6:

index 63bcd90f..5d395487 100644
--- a/src/platform_layers/linux_platform_layer/src/linux_adu_core_impl.cpp
+++ b/src/platform_layers/linux_platform_layer/src/linux_adu_core_impl.cpp
@@ -60,6 +60,11 @@ std::unique_ptr<LinuxPlatformLayer> LinuxPlatformLayer::Create()
 LinuxPlatformLayer::~LinuxPlatformLayer()
 {
     ExtensionManager::Uninit();
+    _IsCancellationRequested = true;
+    if (_activeWorker.joinable())
+    {
+        _activeWorker.join();
+    }
 }

 /**
diff --git a/src/platform_layers/linux_platform_layer/src/linux_adu_core_impl.hpp b/src/platform_layers/linux_platform_layer/src/linux_adu_core_impl.hpp
index 02802f5e..0156a05c 100644
--- a/src/platform_layers/linux_platform_layer/src/linux_adu_core_impl.hpp
+++ b/src/platform_layers/linux_platform_layer/src/linux_adu_core_impl.hpp
@@ -37,6 +37,7 @@ public:
 private:
     static std::string g_componentsInfo;
     static time_t g_lastComponentsCheckTime;
+    static std::thread _activeWorker;

     //
     // Static callbacks.
@@ -72,6 +73,12 @@ private:
         {
             Log_Info("Download thread started.");

+
+            if (_activeWorker.joinable())
+            {
+                _activeWorker.join();  // wait for previous operation
+            }
+
             // Pointers passed to this method are guaranteed to be valid until WorkCompletionCallback is called.
             std::thread worker{ [token, workCompletionData, workflowData] {
                 const ADUC_Result result{ ADUC::ExceptionUtils::CallResultMethodAndHandleExceptions(
@@ -83,9 +90,7 @@ private:
                 workCompletionData->WorkCompletionCallback(
                     workCompletionData->WorkCompletionToken, result, true /* isAsync */);
             } };
-
-            // Allow the thread to work independently of this main thread.
-            worker.detach();
+            _activeWorker = std::move(worker);

             // Indicate that we've spun off a thread to do the actual work.
             return ADUC_Result{ ADUC_Result_Download_InProgress };
@@ -123,6 +128,11 @@ private:
         {
             Log_Info("Backup thread started");

+            if (_activeWorker.joinable())
+            {
+                _activeWorker.join();  // wait for previous operation
+            }
+
             // Pointers passed to this method are guaranteed to be valid until WorkCompletionCallback is called.
             std::thread worker{ [token, workCompletionData, workflowData] {
                 const ADUC_Result result{ ADUC::ExceptionUtils::CallResultMethodAndHandleExceptions(
@@ -134,9 +144,7 @@ private:
                 workCompletionData->WorkCompletionCallback(
                     workCompletionData->WorkCompletionToken, result, true /* isAsync */);
             } };
-
-            // Allow the thread to work independently of this main thread.
-            worker.detach();
+            _activeWorker = std::move(worker);

             // Indicate that we've spun off a thread to do the actual work.
             result = { ADUC_Result_Backup_InProgress };
@@ -176,6 +184,11 @@ private:
         {
             Log_Info("Install thread started");

+            if (_activeWorker.joinable())
+            {
+                _activeWorker.join();  // wait for previous operation
+            }
+
             // Pointers passed to this method are guaranteed to be valid until WorkCompletionCallback is called.
             std::thread worker{ [token, workCompletionData, workflowData] {
                 const ADUC_Result result{ ADUC::ExceptionUtils::CallResultMethodAndHandleExceptions(
@@ -187,9 +200,7 @@ private:
                 workCompletionData->WorkCompletionCallback(
                     workCompletionData->WorkCompletionToken, result, true /* isAsync */);
             } };
-
-            // Allow the thread to work independently of this main thread.
-            worker.detach();
+            _activeWorker = std::move(worker);

             // Indicate that we've spun off a thread to do the actual work.
             result = { ADUC_Result_Install_InProgress };
@@ -228,6 +239,11 @@ private:
         {
             Log_Info("Apply thread started");

+            if (_activeWorker.joinable())
+            {
+                _activeWorker.join();  // wait for previous operation
+            }
+
             // Pointers passed to this method are guaranteed to be valid until WorkCompletionCallback is called.
             std::thread worker{ [token, workCompletionData, workflowData] {
                 const ADUC_Result result{ ADUC::ExceptionUtils::CallResultMethodAndHandleExceptions(
@@ -238,9 +254,7 @@ private:
                 workCompletionData->WorkCompletionCallback(
                     workCompletionData->WorkCompletionToken, result, true /* isAsync */);
             } };
-
-            // Allow the thread to work independently of this main thread.
-            worker.detach();
+            _activeWorker = std::move(worker);

             // Indicate that we've spun off a thread to do the actual work.
             return ADUC_Result{ ADUC_Result_Apply_InProgress };
@@ -277,6 +291,11 @@ private:
         {
             Log_Info("Restore thread started");

+            if (_activeWorker.joinable())
+            {
+                _activeWorker.join();  // wait for previous operation
+            }
+
             // Pointers passed to this method are guaranteed to be valid until WorkCompletionCallback is called.
             std::thread worker{ [token, workCompletionData, workflowData] {
                 const ADUC_Result result{ ADUC::ExceptionUtils::CallResultMethodAndHandleExceptions(
@@ -287,9 +306,7 @@ private:
                 workCompletionData->WorkCompletionCallback(
                     workCompletionData->WorkCompletionToken, result, true /* isAsync */);
             } };
-
-            // Allow the thread to work independently of this main thread.
-            worker.detach();
+            _activeWorker = std::move(worker);

             // Indicate that we've spun off a thread to do the actual work.
             return ADUC_Result{ ADUC_Result_Restore_InProgress };

iot_core_dump_fix.patch

[Nox-MSFT]

Thanks @Silver-Yang for reporting this issue. We'll investigate and address this as soon as we can.