From 915cd91d12957473cf8aecdfc08a0783124eca3f Mon Sep 17 00:00:00 2001 From: David Justo Date: Tue, 5 Sep 2023 10:19:42 -0700 Subject: [PATCH 1/6] patch thread yielding --- src/DurableEngine/SharedMemory.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/DurableEngine/SharedMemory.cs b/src/DurableEngine/SharedMemory.cs index 66093d8..0132789 100644 --- a/src/DurableEngine/SharedMemory.cs +++ b/src/DurableEngine/SharedMemory.cs @@ -38,11 +38,11 @@ internal void Add(OrchestrationAction action) /// public void YieldToInvokerThread() { + // Get user-code thread ready to block. + userCodeThreadTurn.Reset(); + // Wake invoker thread. invokerThreadTurn.Set(); - - // Block user-code thread. - userCodeThreadTurn.Reset(); userCodeThreadTurn.WaitOne(); } @@ -54,12 +54,12 @@ public void YieldToInvokerThread() /// True if the user-code thread completed, False if it requests an API to be awaited. public bool YieldToUserCodeThread(WaitHandle completionHandle) { - // Wake user-code thread - userCodeThreadTurn.Set(); - // Get invoker thread ready to block invokerThreadTurn.Reset(); + // Wake user-code thread + userCodeThreadTurn.Set(); + // Wake up when either the user-code returns, or when we're yielded-to for `await`'ing. var index = WaitHandle.WaitAny(new[] { completionHandle, invokerThreadTurn }); var shouldStop = index == 0; From 839f02b2bf686bfa220a9e4193d560ce2fb059f9 Mon Sep 17 00:00:00 2001 From: David Justo Date: Tue, 5 Sep 2023 11:09:04 -0700 Subject: [PATCH 2/6] remove manual use of reset step, instead leverage the automatic reset step --- src/DurableEngine/SharedMemory.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/DurableEngine/SharedMemory.cs b/src/DurableEngine/SharedMemory.cs index 0132789..feec215 100644 --- a/src/DurableEngine/SharedMemory.cs +++ b/src/DurableEngine/SharedMemory.cs @@ -38,9 +38,6 @@ internal void Add(OrchestrationAction action) /// public void YieldToInvokerThread() { - // Get user-code thread ready to block. - userCodeThreadTurn.Reset(); - // Wake invoker thread. invokerThreadTurn.Set(); userCodeThreadTurn.WaitOne(); @@ -54,9 +51,6 @@ public void YieldToInvokerThread() /// True if the user-code thread completed, False if it requests an API to be awaited. public bool YieldToUserCodeThread(WaitHandle completionHandle) { - // Get invoker thread ready to block - invokerThreadTurn.Reset(); - // Wake user-code thread userCodeThreadTurn.Set(); From 2063938346b82225e99800ccd54c95b04f6acc63 Mon Sep 17 00:00:00 2001 From: David Justo Date: Fri, 8 Sep 2023 11:19:57 -0700 Subject: [PATCH 3/6] patch races --- src/DurableEngine/OrchestrationInvoker.cs | 2 ++ src/DurableEngine/SharedMemory.cs | 13 +------------ src/DurableEngine/Tasks/DurableTask.cs | 5 +---- 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/DurableEngine/OrchestrationInvoker.cs b/src/DurableEngine/OrchestrationInvoker.cs index ae58b15..122d1be 100644 --- a/src/DurableEngine/OrchestrationInvoker.cs +++ b/src/DurableEngine/OrchestrationInvoker.cs @@ -69,6 +69,7 @@ internal Hashtable Invoke(IPowerShellServices powerShellServices) // Start user-code thread, which contains the "actual" PS orchestrator var outputBuffer = new PSDataCollection(); + var asyncResult = powerShellServices.BeginInvoke(outputBuffer); var orchestratorReturnedHandle = asyncResult.AsyncWaitHandle; @@ -109,6 +110,7 @@ internal Hashtable Invoke(IPowerShellServices powerShellServices) await task.GetDTFxTask(); } // Exceptions are ignored at this point, they will be re-surfaced by the PS code if left unhandled. catch { } + context.SharedMemory.userCodeThreadTurn.Set(); } }; diff --git a/src/DurableEngine/SharedMemory.cs b/src/DurableEngine/SharedMemory.cs index 1f3da3b..3a9a516 100644 --- a/src/DurableEngine/SharedMemory.cs +++ b/src/DurableEngine/SharedMemory.cs @@ -5,6 +5,7 @@ namespace DurableEngine { + using System; using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; @@ -51,23 +52,11 @@ public void YieldToInvokerThread() /// True if the user-code thread completed, False if it requests an API to be awaited. public bool YieldToUserCodeThread(WaitHandle completionHandle) { - // Wake user-code thread - userCodeThreadTurn.Set(); // Wake up when either the user-code returns, or when we're yielded-to for `await`'ing. var index = WaitHandle.WaitAny(new[] { completionHandle, invokerThreadTurn }); var shouldStop = index == 0; return shouldStop; } - - /// - /// Blocks user code thread if the orchestrator-invoker thread is currently running. - /// This guarantees that the user-code thread and the orchestration-invoker thread run one - /// at a time after this point. - /// - public void GuaranteeUserCodeTurn() - { - userCodeThreadTurn.WaitOne(); - } } } \ No newline at end of file diff --git a/src/DurableEngine/Tasks/DurableTask.cs b/src/DurableEngine/Tasks/DurableTask.cs index 0e1604a..f796bef 100644 --- a/src/DurableEngine/Tasks/DurableTask.cs +++ b/src/DurableEngine/Tasks/DurableTask.cs @@ -49,10 +49,6 @@ internal OrchestrationAction GetOrCreateAction() /// Function to write an exception to the pipeline. public void Execute(Action write, Action writeErr) { - // Ensure that a DurableTask in the usercode thread - // only executes while the orchestration-invoker thread is blocked. - OrchestrationContext.SharedMemory.GuaranteeUserCodeTurn(); - DurableTask task = this; if (NoWait) @@ -72,6 +68,7 @@ public void Execute(Action write, Action writeErr) OrchestrationContext.SharedMemory.Add(task.GetOrCreateAction()); } + // Signal orchestration thread to await the Task. // This is necessary for DTFx to determine if a result exists for the Task. OrchestrationContext.SharedMemory.YieldToInvokerThread(); From 0fccb0a61d735ed290918927ff36af2992e231cd Mon Sep 17 00:00:00 2001 From: David Justo Date: Fri, 8 Sep 2023 12:01:14 -0700 Subject: [PATCH 4/6] add comments --- src/DurableEngine/OrchestrationInvoker.cs | 9 +++++++-- src/DurableEngine/SharedMemory.cs | 14 +++++++++++--- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/DurableEngine/OrchestrationInvoker.cs b/src/DurableEngine/OrchestrationInvoker.cs index 122d1be..fee9506 100644 --- a/src/DurableEngine/OrchestrationInvoker.cs +++ b/src/DurableEngine/OrchestrationInvoker.cs @@ -77,7 +77,7 @@ internal Hashtable Invoke(IPowerShellServices powerShellServices) while (true) { // block this thread until user-code thread (the PS orchestrator) invokes a DF CmdLet or completes. - var orchestratorReturned = context.SharedMemory.YieldToUserCodeThread(orchestratorReturnedHandle); + var orchestratorReturned = context.SharedMemory.WaitForInvokerThreadTurn(orchestratorReturnedHandle); if (orchestratorReturned) { // The PS orchestrator has a return value, there's no more DF APIs to await. @@ -110,7 +110,12 @@ internal Hashtable Invoke(IPowerShellServices powerShellServices) await task.GetDTFxTask(); } // Exceptions are ignored at this point, they will be re-surfaced by the PS code if left unhandled. catch { } - context.SharedMemory.userCodeThreadTurn.Set(); + + // Wake up user-code thread. For a small moment, both the user code thread and the invoker thread + // will be running at the same time. + // However, the invoker thread will block itself again at the start of the next loop until the user-code + // thread yields control. + context.SharedMemory.WakeUserCodeThread(); } }; diff --git a/src/DurableEngine/SharedMemory.cs b/src/DurableEngine/SharedMemory.cs index 3a9a516..3d950a7 100644 --- a/src/DurableEngine/SharedMemory.cs +++ b/src/DurableEngine/SharedMemory.cs @@ -45,12 +45,11 @@ public void YieldToInvokerThread() } /// - /// Blocks Orchestration-invoker thread, wakes up user-code thread. - /// This is usually used after the invoker has a result for the PS orchestrator. + /// Blocks Orchestration-invoker thread until the user-code thread completes or yields. /// /// The WaitHandle tracking if the user-code thread completed. /// True if the user-code thread completed, False if it requests an API to be awaited. - public bool YieldToUserCodeThread(WaitHandle completionHandle) + public bool WaitForInvokerThreadTurn(WaitHandle completionHandle) { // Wake up when either the user-code returns, or when we're yielded-to for `await`'ing. @@ -58,5 +57,14 @@ public bool YieldToUserCodeThread(WaitHandle completionHandle) var shouldStop = index == 0; return shouldStop; } + + /// + /// Wakes up the user-code thread without blocking the invoker thread. + /// The invoker thread should block itself afterwards to prevent races. + /// + public void WakeUserCodeThread() + { + userCodeThreadTurn.Set(); + } } } \ No newline at end of file From 6dfc16187ff543a6c50f013f5fe49b60f44603fb Mon Sep 17 00:00:00 2001 From: David Justo Date: Fri, 8 Sep 2023 12:06:50 -0700 Subject: [PATCH 5/6] minor refactoring --- src/DurableEngine/Tasks/DurableTask.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/DurableEngine/Tasks/DurableTask.cs b/src/DurableEngine/Tasks/DurableTask.cs index f796bef..1dd4b72 100644 --- a/src/DurableEngine/Tasks/DurableTask.cs +++ b/src/DurableEngine/Tasks/DurableTask.cs @@ -68,7 +68,6 @@ public void Execute(Action write, Action writeErr) OrchestrationContext.SharedMemory.Add(task.GetOrCreateAction()); } - // Signal orchestration thread to await the Task. // This is necessary for DTFx to determine if a result exists for the Task. OrchestrationContext.SharedMemory.YieldToInvokerThread(); From 1aa22b36fff0b96bfb2c191b100ab00a69473712 Mon Sep 17 00:00:00 2001 From: David Justo Date: Fri, 8 Sep 2023 12:38:54 -0700 Subject: [PATCH 6/6] minor refactoring --- src/DurableEngine/OrchestrationInvoker.cs | 1 - src/DurableEngine/SharedMemory.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/src/DurableEngine/OrchestrationInvoker.cs b/src/DurableEngine/OrchestrationInvoker.cs index fee9506..e9841e6 100644 --- a/src/DurableEngine/OrchestrationInvoker.cs +++ b/src/DurableEngine/OrchestrationInvoker.cs @@ -69,7 +69,6 @@ internal Hashtable Invoke(IPowerShellServices powerShellServices) // Start user-code thread, which contains the "actual" PS orchestrator var outputBuffer = new PSDataCollection(); - var asyncResult = powerShellServices.BeginInvoke(outputBuffer); var orchestratorReturnedHandle = asyncResult.AsyncWaitHandle; diff --git a/src/DurableEngine/SharedMemory.cs b/src/DurableEngine/SharedMemory.cs index 3d950a7..c5012a6 100644 --- a/src/DurableEngine/SharedMemory.cs +++ b/src/DurableEngine/SharedMemory.cs @@ -5,7 +5,6 @@ namespace DurableEngine { - using System; using System.Collections.Generic; using System.Threading; using System.Threading.Tasks;