Skip to content

perf: avoid blocking update checks in editor window #954

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.

Sign up for GitHub

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

Open
jiajunfeng wants to merge 1 commit into
base: beta
Choose a base branch
from
+96 −41
Open

perf: avoid blocking update checks in editor window #954

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
41 changes: 39 additions & 2 deletions MCPForUnity/Editor/Services/PackageUpdateService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace MCPForUnity.Editor.Services
/// </summary>
public class PackageUpdateService : IPackageUpdateService
{
private const int DefaultRequestTimeoutMs = 3000;
private const string LastCheckDateKey = EditorPrefKeys.LastUpdateCheck;
private const string CachedVersionKey = EditorPrefKeys.LatestKnownVersion;
private const string LastBetaCheckDateKey = EditorPrefKeys.LastUpdateCheck + ".beta";
Expand Down Expand Up @@ -265,7 +266,7 @@ protected virtual string FetchLatestVersionFromGitHub(string branch)
// - More reliable - doesn't require releases to be published
// - Direct source of truth from the main branch

using (var client = new WebClient())
using (var client = CreateWebClient())
{
client.Headers.Add("User-Agent", "Unity-MCPForUnity-UpdateChecker");
string packageJsonUrl = string.Equals(branch, "beta", StringComparison.OrdinalIgnoreCase)
Expand Down Expand Up @@ -304,7 +305,7 @@ protected virtual string FetchLatestVersionFromAssetStoreJson()
{
try
{
using (var client = new WebClient())
using (var client = CreateWebClient())
{
client.Headers.Add("User-Agent", "Unity-MCPForUnity-AssetStoreUpdateChecker");
string jsonContent = client.DownloadString(AssetStoreVersionUrl);
Expand All @@ -322,5 +323,41 @@ protected virtual string FetchLatestVersionFromAssetStoreJson()
return null;
}
}

protected virtual WebClient CreateWebClient()
{
return new TimeoutWebClient(GetRequestTimeoutMs());
}

protected virtual int GetRequestTimeoutMs()
{
return DefaultRequestTimeoutMs;
}

private sealed class TimeoutWebClient : WebClient
{
private readonly int _timeoutMs;

public TimeoutWebClient(int timeoutMs)
{
_timeoutMs = timeoutMs;
}

protected override WebRequest GetWebRequest(Uri address)
{
var request = base.GetWebRequest(address);
if (request != null)
{
request.Timeout = _timeoutMs;

if (request is HttpWebRequest httpRequest)
{
httpRequest.ReadWriteTimeout = _timeoutMs;
}
}

return request;
}
}
}
}
96 changes: 57 additions & 39 deletions MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ public class MCPForUnityEditorWindow : EditorWindow
private bool toolsLoaded = false;
private bool resourcesLoaded = false;
private double lastRefreshTime = 0;
private const double RefreshDebounceSeconds = 0.5;
private bool updateCheckQueued = false;
private const double RefreshDebounceSeconds = 0.5;
private bool updateCheckQueued = false;
private bool updateCheckInFlight = false;

private enum ActivePanel
{
Expand Down Expand Up @@ -350,53 +351,70 @@ private void UpdateVersionLabel()
: $"MCP For Unity v{version}";
}

private void QueueUpdateCheck()
{
if (updateCheckQueued)
{
return;
}
private void QueueUpdateCheck()
{
if (updateCheckQueued || updateCheckInFlight)
{
return;
}

updateCheckQueued = true;
EditorApplication.delayCall += CheckForPackageUpdates;
}

private void CheckForPackageUpdates()
{
updateCheckQueued = false;
if (updateNotification == null || updateNotificationText == null)
private void CheckForPackageUpdates()
{
updateCheckQueued = false;

if (updateNotification == null || updateNotificationText == null)
{
return;
}

string currentVersion = AssetPathUtility.GetPackageVersion();
if (string.IsNullOrEmpty(currentVersion) || currentVersion == "unknown")
{
updateNotification.RemoveFromClassList("visible");
return;
}

try
{
var result = MCPServiceLocator.Updates.CheckForUpdate(currentVersion);
if (result.CheckSucceeded && result.UpdateAvailable && !string.IsNullOrEmpty(result.LatestVersion))
{
updateNotificationText.text = $"Update available: v{result.LatestVersion} (current: v{currentVersion})";
updateNotificationText.tooltip = $"Latest version: v{result.LatestVersion}\nCurrent version: v{currentVersion}";
updateNotification.AddToClassList("visible");
}
else
{
updateNotification.RemoveFromClassList("visible");
}
}
catch (Exception ex)
{
McpLog.Info($"Package update check skipped: {ex.Message}");
updateNotification.RemoveFromClassList("visible");
}
}
if (string.IsNullOrEmpty(currentVersion) || currentVersion == "unknown")
{
updateNotification.RemoveFromClassList("visible");
return;
}

updateCheckInFlight = true;
Task.Run(() =>
{
try
{
return MCPServiceLocator.Updates.CheckForUpdate(currentVersion);
}
Comment on lines +382 to +387
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the background-thread call path and UnityEditor API usage.
rg -n "Task.Run|CheckForPackageUpdates|CheckForUpdate\\(" MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs -C 3
rg -n "EditorPrefs\\.(GetString|SetString|DeleteKey)" MCPForUnity/Editor/Services/PackageUpdateService.cs -C 2
rg -n "public static IPackageUpdateService Updates =>" MCPForUnity/Editor/Services/MCPServiceLocator.cs -C 2

Repository: CoplayDev/unity-mcp

Length of output: 2267

🏁 Script executed:

# Get the full implementation of CheckForUpdate to see all code paths
rg -n "public.*CheckForUpdate" MCPForUnity/Editor/Services/PackageUpdateService.cs -A 50

Repository: CoplayDev/unity-mcp

Length of output: 2529

🏁 Script executed:

# Check if there's any thread-safety or main-thread queuing already in place
rg -n "EditorApplication\.(delayCall|update)" MCPForUnity/Editor/Services/PackageUpdateService.cs

Repository: CoplayDev/unity-mcp

Length of output: 45

🏁 Script executed:

# Verify the full context of the Task.Run usage in the editor window
cat -n MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs | sed -n '365,400p'

Repository: CoplayDev/unity-mcp

Length of output: 1525

Move EditorPrefs access to the editor thread; only run network I/O on background threads.

Task.Run at line 382 executes MCPServiceLocator.Updates.CheckForUpdate(...) on a worker thread. That method calls EditorPrefs.GetString(...) unconditionally at lines 42–43 (before any network I/O), which is not thread-safe. Additionally, the lazy initialization of MCPServiceLocator.Updates at line 29 happens on the worker thread on first access.

While the result processing correctly uses EditorApplication.delayCall to queue back to the main thread (line 395), the EditorPrefs calls have already executed unsafely. Refactor to read/write the editor cache on the main thread and move only the network fetch (FetchLatestVersionFromGitHub / FetchLatestVersionFromAssetStoreJson) to the background thread.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs` around lines 382 -
387, The CheckForUpdate call currently runs on a worker thread and therefore
performs unsafe EditorPrefs access and lazy initialization of
MCPServiceLocator.Updates off the main thread; instead, ensure
MCPServiceLocator.Updates is accessed on the editor thread and read any
EditorPrefs cache (EditorPrefs.GetString/SetString) on the main thread before
starting background work, then call Task.Run only to execute the network methods
(FetchLatestVersionFromGitHub / FetchLatestVersionFromAssetStoreJson) using the
cached values as parameters; finally, marshal the result back with
EditorApplication.delayCall and perform any EditorPrefs writes or
MCPServiceLocator.Updates lazy initialization there.

catch (Exception ex)
{
McpLog.Info($"Package update check skipped: {ex.Message}");
return null;
}
}).ContinueWith(t =>
{
EditorApplication.delayCall += () =>
{
updateCheckInFlight = false;

if (this == null || updateNotification == null || updateNotificationText == null)
{
return;
}

var result = t.Status == TaskStatus.RanToCompletion ? t.Result : null;
if (result != null && result.CheckSucceeded && result.UpdateAvailable && !string.IsNullOrEmpty(result.LatestVersion))
{
updateNotificationText.text = $"Update available: v{result.LatestVersion} (current: v{currentVersion})";
updateNotificationText.tooltip = $"Latest version: v{result.LatestVersion}\nCurrent version: v{currentVersion}";
updateNotification.AddToClassList("visible");
}
else
{
updateNotification.RemoveFromClassList("visible");
}
};
}, TaskScheduler.Default);
}

private void EnsureToolsLoaded()
{
Expand Down