Skip to content
This repository was archived by the owner on May 24, 2026. It is now read-only.

Extract shared JsonSerializerOptions to eliminate duplicate allocations #774

Open
PureWeen wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Extract shared JsonSerializerOptions to eliminate duplicate allocations #774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions PolyPilot.Tests/PolyPilot.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
<Compile Include="../PolyPilot/Models/AuditLogEntry.cs" Link="Shared/AuditLogEntry.cs" />
<Compile Include="../PolyPilot/Models/BridgeMessages.cs" Link="Shared/BridgeMessages.cs" />
<Compile Include="../PolyPilot/Models/ConnectionSettings.cs" Link="Shared/ConnectionSettings.cs" />
<Compile Include="../PolyPilot/Models/JsonDefaults.cs" Link="Shared/JsonDefaults.cs" />
<Compile Include="../PolyPilot/Models/PlatformHelper.cs" Link="Shared/PlatformHelper.cs" />
<Compile Include="../PolyPilot/Models/PlatformPaths.cs" Link="Shared/PlatformPaths.cs" />
<Compile Include="../PolyPilot/Models/SessionOrganization.cs" Link="Shared/SessionOrganization.cs" />
Expand Down
4 changes: 2 additions & 2 deletions PolyPilot/Models/ConnectionSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,9 +331,9 @@ public void Save()
Directory.CreateDirectory(dir);
#if IOS || ANDROID
SaveMobileSecretsIfDirty();
var json = JsonSerializer.Serialize(this, new JsonSerializerOptions { WriteIndented = true });
var json = JsonSerializer.Serialize(this, JsonDefaults.Indented);
#else
var json = JsonSerializer.Serialize(this, new JsonSerializerOptions { WriteIndented = true });
var json = JsonSerializer.Serialize(this, JsonDefaults.Indented);
#endif
File.WriteAllText(SettingsPath, json);
}
Expand Down
8 changes: 8 additions & 0 deletions PolyPilot/Models/JsonDefaults.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
using System.Text.Json;

namespace PolyPilot.Models;

internal static class JsonDefaults
{
internal static readonly JsonSerializerOptions Indented = new() { WriteIndented = true };
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 MODERATE — Shared instance not hardened against mutation (3/3 reviewers)

readonly only prevents reassignment of the field reference; it does not prevent mutation of the object's properties or Converters list. JsonSerializerOptions is fully mutable from construction until the first JsonSerializer.Serialize() call, at which point System.Text.Json internally freezes it.

Concrete failing scenario: Any future code that calls JsonDefaults.Indented.Converters.Add(...) or sets .PropertyNamingPolicy etc. before the first serialization will silently corrupt every consumer in the assembly. After freeze, the same attempt throws InvalidOperationException from deep inside System.Text.Json with no stack frame pointing back here — a hard-to-diagnose crash.

All current call sites are pure reads so there is no immediate regression, but the field is internal (accessible throughout the assembly) with no compile-time guard.

Fix: Call MakeReadOnly() immediately after construction (.NET 7+, this project targets .NET 10):

internal static class JsonDefaults
{
    internal static readonly JsonSerializerOptions Indented = CreateIndented();

    private static JsonSerializerOptions CreateIndented()
    {
        var opts = new JsonSerializerOptions { WriteIndented = true };
        opts.MakeReadOnly();
        return opts;
    }
}

This causes any mutation attempt to throw immediately and deterministically at the mutation site rather than producing mysterious downstream failures.

}
2 changes: 1 addition & 1 deletion PolyPilot/Services/CopilotService.Events.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1493,7 +1493,7 @@ private static string FormatToolResult(object? result)
if (!string.IsNullOrEmpty(val)) return val;
}
}
var json = JsonSerializer.Serialize(result, new JsonSerializerOptions { WriteIndented = true });
var json = JsonSerializer.Serialize(result, JsonDefaults.Indented);
if (json != "{}" && json != "null") return json;
}
catch { }
Expand Down
4 changes: 2 additions & 2 deletions PolyPilot/Services/CopilotService.Organization.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ private void SaveOrganizationCore()
DeletedRepoGroupRepoIds = new HashSet<string>(Organization.DeletedRepoGroupRepoIds)
};
}
var json = JsonSerializer.Serialize(snapshot, new JsonSerializerOptions { WriteIndented = true });
var json = JsonSerializer.Serialize(snapshot, JsonDefaults.Indented);
WriteOrgFile(json);
}
catch (Exception ex)
Expand Down Expand Up @@ -3056,7 +3056,7 @@ internal void SavePendingOrchestration(PendingOrchestration pending)
try
{
Directory.CreateDirectory(PolyPilotBaseDir);
var json = JsonSerializer.Serialize(pending, new JsonSerializerOptions { WriteIndented = true });
var json = JsonSerializer.Serialize(pending, JsonDefaults.Indented);
var tmp = PendingOrchestrationFile + ".tmp";
File.WriteAllText(tmp, json);
File.Move(tmp, PendingOrchestrationFile, overwrite: true);
Expand Down
6 changes: 3 additions & 3 deletions PolyPilot/Services/CopilotService.Persistence.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ private void WriteActiveSessionsFile(List<ActiveSessionEntry> entries)
Debug($"Failed to merge existing sessions: {ex.Message}");
}

var json = JsonSerializer.Serialize(entries, new JsonSerializerOptions { WriteIndented = true });
var json = JsonSerializer.Serialize(entries, JsonDefaults.Indented);
// Atomic write: write to temp file then rename to prevent corruption on crash
var tempFile = ActiveSessionsFile + ".tmp";
File.WriteAllText(tempFile, json);
Expand Down Expand Up @@ -1458,7 +1458,7 @@ public void SetSessionAlias(string sessionId, string alias)
{
// Ensure directory exists (required on iOS where it may not exist by default)
Directory.CreateDirectory(PolyPilotBaseDir);
var json = JsonSerializer.Serialize(aliases, new JsonSerializerOptions { WriteIndented = true });
var json = JsonSerializer.Serialize(aliases, JsonDefaults.Indented);
File.WriteAllText(SessionAliasesFile, json);
}
catch { }
Expand Down Expand Up @@ -1554,7 +1554,7 @@ public bool DeletePersistedSession(string sessionId)
.ToList();
if (kept.Count != entries.Count)
{
var updatedJson = JsonSerializer.Serialize(kept, new JsonSerializerOptions { WriteIndented = true });
var updatedJson = JsonSerializer.Serialize(kept, JsonDefaults.Indented);
var tempFile = ActiveSessionsFile + ".tmp";
File.WriteAllText(tempFile, updatedJson);
File.Move(tempFile, ActiveSessionsFile, overwrite: true);
Expand Down
2 changes: 1 addition & 1 deletion PolyPilot/Services/CopilotService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1963,7 +1963,7 @@ internal static string[] GetMcpCliArgs()
// Write merged config back to mcp-config.json so the CLI auto-reads it.
// This is more reliable than --additional-mcp-config for persistent servers.
var merged = new Dictionary<string, object> { ["mcpServers"] = allServers };
var json = JsonSerializer.Serialize(merged, new JsonSerializerOptions { WriteIndented = true });
var json = JsonSerializer.Serialize(merged, JsonDefaults.Indented);
File.WriteAllText(configPath, json);
}
catch (Exception ex)
Expand Down
2 changes: 1 addition & 1 deletion PolyPilot/Services/RepoManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ private void Save()
{
var stateFile = StateFile; // resolve once
Directory.CreateDirectory(Path.GetDirectoryName(stateFile)!);
var json = JsonSerializer.Serialize(_state, new JsonSerializerOptions { WriteIndented = true });
var json = JsonSerializer.Serialize(_state, JsonDefaults.Indented);
// Atomic write: write to .tmp then rename, so a crash during write
// doesn't leave repos.json truncated/corrupt.
var tmp = stateFile + ".tmp";
Expand Down
2 changes: 1 addition & 1 deletion PolyPilot/Services/ScheduledTaskService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ internal void SaveTasks()
{
var dir = Path.GetDirectoryName(TasksFilePath)!;
Directory.CreateDirectory(dir);
var json = JsonSerializer.Serialize(snapshot, new JsonSerializerOptions { WriteIndented = true });
var json = JsonSerializer.Serialize(snapshot, JsonDefaults.Indented);
var tempPath = TasksFilePath + "." + Guid.NewGuid().ToString("N") + ".tmp";
File.WriteAllText(tempPath, json);
File.Move(tempPath, TasksFilePath, overwrite: true);
Expand Down
Loading