CoplayDev/unity-mcp

Unity Editor crashing

Closed this issue · 8 comments

Getting random crashes when using Unity (6000.2.0f1), see attached editor crash log:

mcp crashes.txt

Here's what RooCode (AI) said:

Based on the crash logs and examining the MCPForUnityBridge.cs file, I can identify several potential causes for the Unity crashes you're experiencing:

Root Cause Analysis

The crash is occurring during Unity's assembly reload process, specifically when the MCP bridge is shutting down. Here are the key issues:

1. Race Condition in Stop Method

The Stop() method (lines 328-353) has a potential race condition:

private static void Stop()
{
    lock (startStopLock)  // Good, has locking
    {
        if (!isRunning) return;
        
        // This sets isRunning = false immediately
        isRunning = false;
        
        // But background tasks might still be running...
        WriteHeartbeat(false);  // File I/O during shutdown
        listener?.Stop();       // TCP cleanup
        listener = null;
        
        // This could be called while ProcessCommands is executing
        EditorApplication.update -= ProcessCommands;
        
        Debug.Log("...stopped.");  // Line 346 where crash occurs
    }
}

2. Background Task Management

The code starts a background task with Task.Run(ListenerLoop) (line 314) but never properly cancels or awaits it during shutdown. This can cause:

  • Lingering TCP connections
  • Background threads continuing to execute during domain reload
  • Memory access violations when the domain is being unloaded

3. File I/O During Shutdown

The WriteHeartbeat() calls during shutdown (lines 342, 693) perform file operations that can conflict with Unity's domain reload process.

4. Event Handler Cleanup

The EditorApplication.update -= ProcessCommands (line 345) can be problematic if ProcessCommands is currently executing when the delegate is removed.

Specific Crash Pattern

Your logs show this sequence:

  1. StopAssetImportingV2 triggers asset pipeline refresh
  2. MCP bridge receives OnBeforeAssemblyReload() callback
  3. Stop() method executes and logs at line 346
  4. Native crash occurs during the shutdown sequence

Recommendations to Fix

The script is likely contributing to crashes due to improper cleanup during assembly reloads. Here are the main fixes needed:

  1. Add proper task cancellation - Use CancellationTokenSource to cancel background tasks
  2. Ensure thread safety - The background ListenerLoop task needs proper cancellation handling
  3. Remove file I/O during shutdown - Move heartbeat writes to a safer location
  4. Add exception handling - Wrap cleanup operations in try-catch blocks
  5. Use proper async/await patterns - Replace Task.Run with more controlled task management

The native crash indicates memory corruption or access violations, likely caused by the background task continuing to run while the Unity domain is being unloaded. This is a classic issue with Unity editor scripts that don't properly handle domain reload events.

@gravitydeepimpactsun thanks for the report.

Can I ask a bit more? What kinds of operations are you doing when the crashes happen? Is it random or linked to a certain kind of thing you're trying to have the editor do. I see a texture atlas mentioned in the logs for example.if you could send a screenshot of the commands you gave the LLM just before it crashes that could help too. Thanks!

@dsarno I just open Unity, click Play, the editor will then (usually) crash, it's the same error as shown in the crash log each time.

If it doesn't crash then everything works including UnityMCP.

RooCode suggested fix (untested):

Here's a proposed fix for the crash issues in the MCPForUnityBridge script:

Key Changes Needed

1. Add Task Cancellation Support

// Add these fields at the top of the class
private static CancellationTokenSource cts = new();
private static Task listenerTask = null;

2. Fix the Start Method

public static void Start()
{
    lock (startStopLock)
    {
        // ... existing code ...
        
        // Replace Task.Run with controlled task
        cts = new CancellationTokenSource();
        listenerTask = ListenerLoop(cts.Token);
        
        // ... rest of existing code ...
    }
}

3. Fix the Stop Method

public static void Stop()
{
    lock (startStopLock)
    {
        if (!isRunning)
        {
            return;
        }

        try
        {
            // Mark as stopping early
            isRunning = false;
            
            // Cancel background task first
            cts?.Cancel();
            
            // Wait for task completion with timeout
            if (listenerTask != null)
            {
                Task.WaitAny(listenerTask, Task.Delay(1000));
            }
            
            // Safe cleanup
            if (listener != null)
            {
                try { listener.Stop(); } catch { }
                listener = null;
            }
            
            // Remove update callback safely
            EditorApplication.update -= ProcessCommands;
            
            Debug.Log("<b><color=#2EA3FF>MCP-FOR-UNITY</color></b>: MCPForUnityBridge stopped.");
        }
        catch (Exception ex)
        {
            Debug.LogError($"Error stopping MCPForUnityBridge: {ex.Message}");
        }
    }
}

4. Update ListenerLoop to Support Cancellation

private static async Task ListenerLoop(CancellationToken token)
{
    while (isRunning && !token.IsCancellationRequested)
    {
        try
        {
            // Use cancellable accept
            var acceptTask = listener.AcceptTcpClientAsync();
            var completedTask = await Task.WhenAny(acceptTask, Task.Delay(-1, token));
            
            if (completedTask == acceptTask)
            {
                TcpClient client = await acceptTask;
                _ = HandleClientAsync(client, token);
            }
        }
        catch (OperationCanceledException)
        {
            break;
        }
        catch (ObjectDisposedException)
        {
            break;
        }
        catch (Exception ex)
        {
            if (isRunning && !token.IsCancellationRequested)
            {
                Debug.LogError($"Listener error: {ex.Message}");
            }
        }
    }
}

5. Remove File I/O During Shutdown

private static void OnBeforeAssemblyReload()
{
    // Remove WriteHeartbeat call during shutdown
    try { Stop(); } catch { }
    LogBreadcrumb("Reload");
}

Why This Fixes the Crash

  1. Eliminates Race Conditions: Proper task cancellation ensures background threads stop before domain reload
  2. Safe Resource Cleanup: TCP listener and delegates are cleaned up safely with exception handling
  3. Prevents Memory Corruption: No lingering threads accessing destroyed objects during domain reload
  4. Removes Shutdown File I/O: File operations during shutdown can conflict with Unity's reload process

This should eliminate the native crashes you were experiencing during asset pipeline refreshes and assembly reloads.

Should be fixed in 3.1.0. Please let us know if you see an improved experience.

Should be fixed in 3.1.0. Please let us know if you see an improved experience.

Tried 3.2.1 and it's still crashing.

@gravitydeepimpactsun have you tried in any other version of Unity? I notice that’s an early beta you mentioned.

Hi, this should finally be fixed in the most recent update. Give it a try and let us know!

I have updated my Unity version to 6000.2.2 (fixes some reported Mono crashes in 6000.2.0) and updated unitymcp to 3.3.2 and not had any crashes yet but I can't properly test it yet due to the WinErr 10054 connection bug.

Edit: Updated to 3.4.0 and all working now.