Skip to content

fix concurrent issue #1104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 28, 2025
Merged

fix concurrent issue #1104

merged 1 commit into from
Jul 28, 2025

Conversation

JackJiang1234
Copy link
Contributor

@JackJiang1234 JackJiang1234 commented Jul 21, 2025

User description

Operations that change non-concurrent collections must have exclusive access


PR Type

Bug fix


Description

  • Fix concurrent access issue in agent template rendering

  • Replace direct dictionary modification with local copy

  • Prevent race conditions in multi-threaded scenarios


Diagram Walkthrough

flowchart LR
  A["Agent.TemplateDict"] --> B["Create Local Copy"]
  B --> C["renderDict"]
  C --> D["Modify Local Copy"]
  D --> E["Pass to Render"]
Loading

File Walkthrough

Relevant files
Bug fix
AgentService.Rendering.cs
Fix concurrent dictionary access in rendering                       

src/Infrastructure/BotSharp.Core/Agents/Services/AgentService.Rendering.cs

  • Create local copy of agent.TemplateDict as renderDict
  • Modify local dictionary instead of shared agent dictionary
  • Pass local dictionary to render method
  • Add missing newline at end of file
+5/-4     

Operations that change non-concurrent collections must have exclusive access
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Thread Safety

While the fix prevents modification of the shared agent.TemplateDict, the Agent object itself and its TemplateDict property may still be accessed concurrently by other threads. Consider if additional synchronization is needed for the Agent object or if this fix fully addresses all concurrent access scenarios.

var renderDict = new Dictionary<string, object>(agent.TemplateDict);
foreach (var t in conv.States.GetStates())
{
     renderDict[t.Key] = t.Value;
}

renderDict[TemplateRenderConstant.RENDER_AGENT] = agent;
var res = render.Render(string.Join("\r\n", instructions), renderDict);

Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Fix inconsistent indentation

The indentation is inconsistent with an extra space before renderDict[t.Key] =
t.Value;. This should align with the standard indentation used throughout the
method.

src/Infrastructure/BotSharp.Core/Agents/Services/AgentService.Rendering.cs [20-24]

 var renderDict = new Dictionary<string, object>(agent.TemplateDict);
 foreach (var t in conv.States.GetStates())
 {
-     renderDict[t.Key] = t.Value;
+    renderDict[t.Key] = t.Value;
 }
  • Apply / Chat
Suggestion importance[1-10]: 3

__

Why: The suggestion correctly identifies an inconsistent indentation on line 23, which is a minor but valid code style improvement for readability.

Low
  • More

@yileicn
Copy link
Collaborator

yileicn commented Jul 21, 2025

reviewed

@Oceania2018 Oceania2018 requested a review from iceljc July 28, 2025 15:28
@Oceania2018 Oceania2018 merged commit bf65265 into SciSharp:master Jul 28, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants