-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix: reliable auto-start and multi-instance connection support #1121
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
base: beta
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -381,14 +381,12 @@ public static void Start() | |
| private static TcpListener CreateConfiguredListener(int port) | ||
| { | ||
| var newListener = new TcpListener(IPAddress.Loopback, port); | ||
| #if UNITY_EDITOR_OSX | ||
| // SO_REUSEADDR is intentionally NOT set. On macOS it allows multiple | ||
| // processes (including AssetImportWorkers) to bind the same port, | ||
| // causing connections to land on a worker that can't process commands. | ||
| // The ExclusiveAddressUse flag prevents this; port-busy conflicts are | ||
| // handled by the retry/fallback logic in Start() and the reload handler. | ||
| // SO_REUSEADDR allows multiple processes to bind the same port on | ||
| // Linux/macOS, causing connections to land on a worker/old instance | ||
| // that can't process commands. ExclusiveAddressUse prevents this; | ||
| // port-busy conflicts are handled by the retry/fallback logic in | ||
| // Start() and the reload handler. | ||
| try { newListener.Server.ExclusiveAddressUse = true; } catch { } | ||
| #endif | ||
| try | ||
| { | ||
| newListener.Server.LingerState = new LingerOption(true, 0); | ||
|
|
@@ -459,11 +457,25 @@ public static void Stop() | |
| { | ||
| dir = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".unity-mcp"); | ||
| } | ||
| string statusFile = Path.Combine(dir, $"unity-mcp-status-{ComputeProjectHash(Application.dataPath)}.json"); | ||
| if (File.Exists(statusFile)) | ||
| string projectHash = ComputeProjectHash(Application.dataPath); | ||
|
|
||
| string projectFile = Path.Combine(dir, $"unity-mcp-status-{projectHash}.json"); | ||
| if (File.Exists(projectFile)) | ||
| { | ||
| File.Delete(projectFile); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift Do not delete another instance's project status file. The project-scoped file is shared. Line 1141 lets any running instance replace it. If instance B writes it last, line 465 lets instance A delete B's active endpoint when A stops. Clients that use the project-scoped discovery file cannot find B until its next heartbeat. Coordinate project-file ownership across processes. For example, maintain a project index from PID-scoped files under an interprocess lock, then update or delete the project file only from that coordinated state. 🤖 Prompt for AI Agents |
||
| if (IsDebugEnabled()) McpLog.Info($"Deleted project status file: {projectFile}"); | ||
| } | ||
|
|
||
| // Clean up instance-specific file so stale entries don't accumulate. | ||
| int pid = s_CachedProcessId; | ||
| if (pid > 0) | ||
| { | ||
| File.Delete(statusFile); | ||
| if (IsDebugEnabled()) McpLog.Info($"Deleted status file: {statusFile}"); | ||
| string instanceFile = Path.Combine(dir, $"unity-mcp-status-{projectHash}-{pid}.json"); | ||
| if (File.Exists(instanceFile)) | ||
| { | ||
| File.Delete(instanceFile); | ||
| if (IsDebugEnabled()) McpLog.Info($"Deleted instance status file: {instanceFile}"); | ||
| } | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
|
|
@@ -1065,6 +1077,13 @@ private static bool IsValidJson(string text) | |
| } | ||
|
|
||
|
|
||
| private static readonly int s_CachedProcessId = GetProcessId(); | ||
|
|
||
| private static int GetProcessId() | ||
| { | ||
| try { return Process.GetCurrentProcess().Id; } catch { return 0; } | ||
| } | ||
|
|
||
| public static void WriteHeartbeat(bool reloading, string reason = null) | ||
| { | ||
| try | ||
|
|
@@ -1075,7 +1094,6 @@ public static void WriteHeartbeat(bool reloading, string reason = null) | |
| dir = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".unity-mcp"); | ||
| } | ||
| Directory.CreateDirectory(dir); | ||
| string filePath = Path.Combine(dir, $"unity-mcp-status-{ComputeProjectHash(Application.dataPath)}.json"); | ||
|
|
||
| string projectName = "Unknown"; | ||
| try | ||
|
|
@@ -1105,6 +1123,7 @@ public static void WriteHeartbeat(bool reloading, string reason = null) | |
| var payload = new | ||
| { | ||
| unity_port = currentUnityPort, | ||
| pid = s_CachedProcessId, | ||
| reloading, | ||
| reason = reason ?? (reloading ? "reloading" : "ready"), | ||
| seq = heartbeatSeq, | ||
|
|
@@ -1114,7 +1133,21 @@ public static void WriteHeartbeat(bool reloading, string reason = null) | |
| last_heartbeat = DateTime.UtcNow.ToString("O"), | ||
| project_scoped_tools = projectScopedTools | ||
| }; | ||
| File.WriteAllText(filePath, JsonConvert.SerializeObject(payload), new System.Text.UTF8Encoding(false)); | ||
| string json = JsonConvert.SerializeObject(payload); | ||
| byte[] utf8Bytes = new System.Text.UTF8Encoding(false).GetBytes(json); | ||
|
|
||
| // Project-scoped file: used by clients that look for any instance of this project. | ||
| string projectFile = Path.Combine(dir, $"unity-mcp-status-{ComputeProjectHash(Application.dataPath)}.json"); | ||
| File.WriteAllBytes(projectFile, utf8Bytes); | ||
|
|
||
| // Instance-scoped file: allows clients to discover and select specific | ||
| // instances when multiple copies of the same project are running. | ||
| if (s_CachedProcessId > 0) | ||
| { | ||
| string instanceFile = Path.Combine(dir, | ||
| $"unity-mcp-status-{ComputeProjectHash(Application.dataPath)}-{s_CachedProcessId}.json"); | ||
| File.WriteAllBytes(instanceFile, utf8Bytes); | ||
| } | ||
| } | ||
| catch (Exception) | ||
| { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge CoplayDev/unity-mcp /tmp/coderabbit-repo-knowledge/coplaydev-unity-mcp-ac22bd92/conventions /tmp/coderabbit-repo-knowledge/coplaydev-unity-mcp-ac22bd92/learningsLength of output: 9175
🏁 Script executed:
Repository: CoplayDev/unity-mcp
Length of output: 9759
🏁 Script executed:
Repository: CoplayDev/unity-mcp
Length of output: 9967
🌐 Web query:
TcpListener(IPAddress, int) ArgumentOutOfRangeException port greater than 65535 Unity .NET API💡 Result:
Yes, the TcpListener(IPAddress, int) constructor throws an ArgumentOutOfRangeException if the provided port is greater than 65535 [1]. In the.NET API, the TcpListener constructor validates the port number to ensure it is within the valid range for TCP ports [2][3]. The internal implementation uses a helper method, typically TcpValidationHelpers.ValidatePortNumber(int port), which checks that the port is between the minimum and maximum allowable values [2][4]. Since TCP ports are represented by a 16-bit unsigned integer, the maximum valid port number is 65535. Any integer value exceeding this limit will trigger an ArgumentOutOfRangeException [1][2]. This behavior is consistent across.NET implementations used in Unity, as Unity's.NET environment relies on standard.NET base class libraries. If you need the system to assign an available port automatically, you can pass 0 as the port argument instead of a high number, which tells the underlying service provider to assign an ephemeral port [1][5].
Citations:
Reject stored ports above 65535 before calling
TcpListener.A persisted positive
unity_portabove65535reachesTcpListener. Its constructor throwsArgumentOutOfRangeException, whichIsPortAvailabledoes not catch. This exception can stop bridge startup instead of returningDefaultPort. Validate the full TCP range and returnfalsefor invalid values inIsPortAvailable, including calls fromSetPreferredPort.🤖 Prompt for AI Agents