Merge pull request #1216 from iceljc/bugfix/refine-code-lock

Bugfix/refine code lock

Author: iceljc

src/Infrastructure/BotSharp.Abstraction/Coding/ICodeProcessor.cs

@@ -17,7 +17,7 @@ public interface ICodeProcessor
     /// <param name="cancellationToken">The cancellation token</param>
     /// <returns></returns>
     /// <exception cref="NotImplementedException"></exception>
-    Task<CodeInterpretResponse> RunAsync(string codeScript, CodeInterpretOptions? options = null, CancellationToken cancellationToken = default)
+    CodeInterpretResponse Run(string codeScript, CodeInterpretOptions? options = null, CancellationToken cancellationToken = default)
         => throw new NotImplementedException();
 
     /// <summary>

src/Infrastructure/BotSharp.Abstraction/Coding/Settings/CodingSettings.cs

@@ -19,5 +19,4 @@ public class CodeScriptExecutionSettings
     public bool UseLock { get; set; }
     public bool UseProcess { get; set; }
     public int TimeoutSeconds { get; set; } = 3;
-    public int MaxConcurrency { get; set; } = 1;
 }

src/Infrastructure/BotSharp.Core.Rules/Engines/RuleEngine.cs

@@ -140,7 +140,7 @@ private async Task<bool> TriggerCodeScript(Agent agent, string triggerName, Rule
 
             var (useLock, useProcess, timeoutSeconds) = GetCodeExecutionConfig();
             using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(timeoutSeconds));
-            var response = await processor.RunAsync(codeScript.Content, options: new()
+            var response = processor.Run(codeScript.Content, options: new()
             {
                 ScriptName = scriptName,
                 Arguments = arguments,

src/Infrastructure/BotSharp.Core/Coding/CodeScriptExecutor.cs

@@ -13,7 +13,5 @@ public void RegisterDI(IServiceCollection services, IConfiguration config)
         var coding = new CodingSettings();
         config.Bind("Coding", coding);
         services.AddSingleton(provider => coding);
-
-        services.AddSingleton<CodeScriptExecutor>();
     }
 }

src/Infrastructure/BotSharp.Core/Coding/CodingPlugin.cs

@@ -69,7 +69,6 @@ public void RegisterDI(IServiceCollection services, IConfiguration config)
         services.AddScoped<ITokenStatistics, TokenStatistics>();
 
         services.AddScoped<IAgentUtilityHook, WebSearchUtilityHook>();
-        services.AddSingleton<CodeScriptExecutor>();
     }
 
     public bool AttachMenu(List<PluginMenuDef> menu)

src/Infrastructure/BotSharp.Core/Conversations/ConversationPlugin.cs

@@ -258,7 +258,7 @@ await hook.OnResponseGenerated(new InstructResponseModel
         // Run code script
         var (useLock, useProcess, timeoutSeconds) = GetCodeExecutionConfig(codingSettings);
         using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(timeoutSeconds));
-        var codeResponse = await codeProcessor.RunAsync(context.CodeScript?.Content ?? string.Empty, options: new()
+        var codeResponse = codeProcessor.Run(context.CodeScript?.Content ?? string.Empty, options: new()
         {
             ScriptName = context.CodeScript?.Name,
             Arguments = context.Arguments,

src/Infrastructure/BotSharp.Core/Instructs/Services/InstructService.Execute.cs

@@ -71,7 +71,7 @@ public async Task<bool> Execute(RoleDialogModel message)
 
         try
         {
-            var (isSuccess, result) = await InnerRunCode(ret.PythonCode);
+            var (isSuccess, result) = InnerRunCode(ret.PythonCode);
             if (isSuccess)
             {
                 message.Content = result;
@@ -107,7 +107,7 @@ public async Task<bool> Execute(RoleDialogModel message)
     /// </summary>
     /// <param name="codeScript"></param>
     /// <returns></returns>
-    private async Task<(bool, string)> InnerRunCode(string codeScript)
+    private (bool, string) InnerRunCode(string codeScript)
     {
         var codeProvider = _codingSettings.CodeExecution?.Processor;
         codeProvider = !string.IsNullOrEmpty(codeProvider) ? codeProvider : BuiltInCodeProcessor.PyInterpreter;
@@ -122,7 +122,7 @@ public async Task<bool> Execute(RoleDialogModel message)
         var (useLock, useProcess, timeoutSeconds) = GetCodeExecutionConfig();
         using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(timeoutSeconds));
 
-        var response = await processor.RunAsync(codeScript, options: new()
+        var response = processor.Run(codeScript, options: new()
         {
             UseLock = useLock,
             UseProcess = useProcess

src/Plugins/BotSharp.Plugin.PythonInterpreter/Functions/PyProgrammerFn.cs

@@ -12,42 +12,29 @@ public class PyCodeInterpreter : ICodeProcessor
 {
     private readonly IServiceProvider _services;
     private readonly ILogger<PyCodeInterpreter> _logger;
-    private readonly CodeScriptExecutor _executor;
     private readonly CodingSettings _settings;
+    private static SemaphoreSlim _semLock = new(initialCount: 1, maxCount: 1);
 
     public PyCodeInterpreter(
         IServiceProvider services,
         ILogger<PyCodeInterpreter> logger,
-        CodeScriptExecutor executor,
         CodingSettings settings)
     {
         _services = services;
         _logger = logger;
-        _executor = executor;
         _settings = settings;
     }
 
     public string Provider => BuiltInCodeProcessor.PyInterpreter;
 
-    public async Task<CodeInterpretResponse> RunAsync(string codeScript, CodeInterpretOptions? options = null, CancellationToken cancellationToken = default)
+    public CodeInterpretResponse Run(string codeScript, CodeInterpretOptions? options = null, CancellationToken cancellationToken = default)
     {
         if (options?.UseLock == true)
         {
-            try
-            {
-                return await _executor.ExecuteAsync(async () =>
-                {
-                    return await InnerRunCode(codeScript, options, cancellationToken);
-                }, cancellationToken: cancellationToken);
-            }
-            catch (Exception ex)
-            {
-                _logger.LogError(ex, "Error when using code script executor.");
-                return new() { ErrorMsg = ex.Message };
-            }
+            return InnerRunWithLock(codeScript, options, cancellationToken);
         }
 
-        return await InnerRunCode(codeScript, options, cancellationToken);
+        return InnerRunCode(codeScript, options, cancellationToken);
     }
 
     public async Task<CodeGenerationResult> GenerateCodeScriptAsync(string text, CodeGenerationOptions? options = null)
@@ -105,7 +92,30 @@ public async Task<CodeGenerationResult> GenerateCodeScriptAsync(string text, Cod
 
 
     #region Private methods
-    private async Task<CodeInterpretResponse> InnerRunCode(string codeScript, CodeInterpretOptions? options = null, CancellationToken cancellationToken = default)
+    private CodeInterpretResponse InnerRunWithLock(string codeScript, CodeInterpretOptions? options = null, CancellationToken cancellationToken = default)
+    {
+        var lockAcquired = false;
+        try
+        {
+            _semLock.Wait(cancellationToken);
+            lockAcquired = true;
+            return InnerRunCode(codeScript, options, cancellationToken);
+        }
+        catch (Exception ex)
+        {
+            _logger.LogError(ex, $"Error in {nameof(InnerRunWithLock)}");
+            return new() { ErrorMsg = ex.Message };
+        }
+        finally
+        {
+            if (lockAcquired)
+            {
+                _semLock.Release();
+            }
+        }
+    }
+
+    private CodeInterpretResponse InnerRunCode(string codeScript, CodeInterpretOptions? options = null, CancellationToken cancellationToken = default)
     {
         var response = new CodeInterpretResponse();
         var scriptName = options?.ScriptName ?? codeScript.SubstringMax(30);
@@ -116,11 +126,11 @@ private async Task<CodeInterpretResponse> InnerRunCode(string codeScript, CodeIn
 
             if (options?.UseProcess == true)
             {
-                response = await CoreRunProcess(codeScript, options, cancellationToken);
+                response = CoreRunProcess(codeScript, options, cancellationToken).ConfigureAwait(false).GetAwaiter().GetResult();
             }
             else
             {
-                response = await CoreRunScript(codeScript, options, cancellationToken);
+                response = CoreRunScript(codeScript, options, cancellationToken);
             }
 
             _logger.LogWarning($"End running python code script in {Provider}: {scriptName}");
@@ -141,7 +151,7 @@ private async Task<CodeInterpretResponse> InnerRunCode(string codeScript, CodeIn
         }
     }
 
-    private async Task<CodeInterpretResponse> CoreRunScript(string codeScript, CodeInterpretOptions? options = null, CancellationToken cancellationToken = default)
+    private CodeInterpretResponse CoreRunScript(string codeScript, CodeInterpretOptions? options = null, CancellationToken cancellationToken = default)
     {
         cancellationToken.ThrowIfCancellationRequested();
 
@@ -161,21 +171,6 @@ private async Task<CodeInterpretResponse> CoreRunScript(string codeScript, CodeI
 
                 try
                 {
-                    // Capture the Python thread ID for the current thread executing under the GIL
-                    var pythonThreadId = PythonEngine.GetPythonThreadID();
-                    using var reg = cancellationToken.Register(() =>
-                    {
-                        try
-                        {
-                            PythonEngine.Interrupt(pythonThreadId);
-                            _logger.LogWarning($"Cancellation requested: issued PythonEngine.Interrupt for thread {pythonThreadId} (request {requestId})");
-                        }
-                        catch (Exception ex)
-                        {
-                            _logger.LogError(ex, "Failed to interrupt Python execution on cancellation.");
-                        }
-                    });
-
                     // Redirect standard output/error to capture it
                     dynamic outIO = io.StringIO();
                     dynamic errIO = io.StringIO();
@@ -206,8 +201,6 @@ private async Task<CodeInterpretResponse> CoreRunScript(string codeScript, CodeI
                     }
                     sys.argv = list;
 
-                    cancellationToken.ThrowIfCancellationRequested();
-
                     // Execute Python script
                     PythonEngine.Exec(codeScript, globals);
 
@@ -217,8 +210,6 @@ private async Task<CodeInterpretResponse> CoreRunScript(string codeScript, CodeI
                     var stdout = outIO.getvalue()?.ToString() as string;
                     var stderr = errIO.getvalue()?.ToString() as string;
 
-                    cancellationToken.ThrowIfCancellationRequested();
-
                     return new CodeInterpretResponse
                     {
                         Result = stdout?.TrimEnd('\r', '\n') ?? string.Empty,
@@ -237,10 +228,10 @@ private async Task<CodeInterpretResponse> CoreRunScript(string codeScript, CodeI
                     sys.stderr = sys.__stderr__;
                     sys.argv = new PyList();
                 }
-            };
+            }
         }, cancellationToken);
 
-        return await execTask.WaitAsync(cancellationToken);
+        return execTask.WaitAsync(cancellationToken).ConfigureAwait(false).GetAwaiter().GetResult();
     }
 
 

src/Plugins/BotSharp.Plugin.PythonInterpreter/Services/PyCodeInterpreter.cs

@@ -527,8 +527,7 @@
       "Processor": null,
       "UseLock": false,
       "UseProcess": false,
-      "TimeoutSeconds": 3,
-      "MaxConcurrency": 1
+      "TimeoutSeconds": 3
     }
   },