Skip to content

CLI server: unsynchronized cliInvocations list races on concurrent commands and leaks raw JSON onto the SSE stream #4831

Description

@mhijaziB

A customer reported an intermittent Unity console warning coming from the CLI local server:

CLI received a message over the local-server that did not start with the expected 'data: ' format.
line=[{"type":"ArgumentOutOfRangeException","stack":"   at cli.Services.HttpServer.ServerService.HandleExec(...) ServerService.cs:line 377
   at cli.Services.HttpServer.ServerService.HandleRequest(...) ServerService.cs:line 294","message":"Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')"}]
UnityEngine.Debug:LogWarning (object)
Beamable.Editor.BeamCli.BeamWebCommand/<RunHttpRequestOnce>d__14:MoveNext () (BeamWebCommand.cs:512)

They asked how to track down which command causes it. The answer is that no command causes it. It is a data race inside the CLI server itself, and any two concurrent invocations finishing at the same instant can trip it.

All references are cli/cli/Services/HttpServer/ServerService.cs at CLI 7.2.3. The line numbers in the customer's stack match the tag and main exactly.

Root cause: cliInvocations is a plain static List<string> shared across request threads

L341:

public static List<string> cliInvocations = new List<string>();

Three call sites touch it, none under a lock:

  • HandleExec L350 adds the raw request body when a command starts.
  • HandleExec L377 removes it in a finally block when the command ends.
  • HandleInfo L337 hands the live list to the JSON serializer for the /info route.

Each HTTP request runs on its own thread-pool task (the server is explicitly designed for concurrent in-flight requests, see _inflightRequests). List<T>.Remove is IndexOf followed by RemoveAt. When two commands complete together, thread A's index goes stale between those two calls because thread B shrank the list, and RemoveAt throws exactly the message above. Concurrent Add calls can also lose entries or corrupt the backing array, and /info enumerating during a mutation would throw InvalidOperationException instead. The customer hit the Remove variant; the others are the same bug wearing different exceptions.

The Unity editor routinely fires several CLI commands at once on domain reload and when windows open, so this is expected to fire occasionally and unpredictably in normal use.

The list was introduced in #3783 (Nov 2024) and has not been touched since.

Why it surfaces as a malformed SSE line

The exception escapes HandleExec into the catch in HandleRequest (L309). That handler serializes a ServerErrorResponse and writes it straight to resp.OutputStream (L318). By that point the response is already an open text/event-stream (header set at L346) and the reporter service has been writing data: lines to the same stream. So the client receives a bare JSON object with no data: prefix, and BeamWebCommand.RunHttpRequestOnce logs the warning and skips the line.

Two secondary problems in the same catch block, both minor:

  • It sets resp.StatusCode = 500 (L319) after headers and body have already gone out. That is a no-op at best.
  • Any exception thrown after streaming has started will surface this same way. Post-stream errors should go through the reporter as a data: line so the client can parse them.

Impact

Cosmetic, and correctly reported by the customer as very low priority. The exception fires in finally after the command has already run and streamed its results, so the command outcome is unaffected. The visible effects are the warning and a stale entry left in cliInvocations, which then shows up as a phantom entry in beam server ps inflightCommands.

Fix

Small and contained, one file:

  1. Make cliInvocations thread-safe. A ConcurrentDictionary<Guid, string> keyed per invocation is the cleanest, since it also stops two identical concurrent command lines from removing each other's entry. A plain lock around the three sites plus a snapshot (ToList()) in HandleInfo is the minimal alternative.
  2. In HandleRequest, once the response has entered streaming mode, report exceptions through the IDataReporterService as a data: line instead of writing raw JSON, and stop setting the status code after headers are sent.

Acceptance

  • A test that runs N concurrent /exec requests against ServerService and asserts no exception escapes HandleExec and cliInvocations is empty afterwards.
  • /info under concurrent load returns a consistent snapshot without throwing.
  • No did not start with the expected 'data: ' warnings in the Unity console across repeated domain reloads with several Beamable windows open.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions