Skip to content

fix(SimpleWebTransport): Guard Queue.TryDequeue behind UNITY_2022_3_OR_NEWER#4105

Merged
miwarnec merged 3 commits intomasterfrom
copilot/fix-queue-trydequeue-compatibility
Mar 26, 2026
Merged

fix(SimpleWebTransport): Guard Queue.TryDequeue behind UNITY_2022_3_OR_NEWER#4105
miwarnec merged 3 commits intomasterfrom
copilot/fix-queue-trydequeue-compatibility

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 26, 2026

Ports 6 upstream fixes into Mirror's vendored Assets/Mirror/Transports/SimpleWeb/SimpleWeb/ addressing buffer memory leaks, thread-safety races, and unbounded send queue growth.

Changes

  • SendAll early exit (SimpleWebServer.cs): Skip buffer allocation when connectionIds.Count == 0.

  • Buffer release on missing connection (WebSocketServer.cs): Call buffer.Release() in Send() when the target connection is not found.

  • Auto-dispose buffers (SimpleWebClient.cs, SimpleWebServer.cs, SendLoop.cs): Replace manual Release() calls with using to guarantee release on exception paths.

  • Thread-safe QueueSend (Connection.cs, SendLoop.cs, WebSocketServer.cs, WebSocketClientStandAlone.cs): Gate all enqueues behind a disposedLock to prevent sending to a disposed connection. SendLoop now captures queue refs via GetSendQueue() once at start.

    // Before
    conn.sendQueue.Enqueue(buffer);
    conn.sendPending.Set();
    // After
    conn.QueueSend(buffer); // release + warn if already disposed
  • Fragment buffer cleanup (ReceiveLoop.cs): Hoist Queue<ArrayBuffer> fragments to Loop scope (eliminates per-message allocation) and drain it in the finally block to release buffers when the loop exits mid-fragmented-message.

  • maxSendQueueSize limit (SimpleWebTransport.cs, SimpleWebServer.cs, WebSocketServer.cs, Connection.cs, WebSocketClientStandAlone.cs): New serialized field (default 1000) threads through to Connection.QueueSend. When the per-connection queue exceeds the limit the buffer is dropped and the connection is kicked, preventing unbounded memory growth on slow clients.

Original prompt

SimpleWebTransport: Port memory leak and thread safety fixes from upstream (James-Frowen/SimpleWebTransport#22)

Port the 6 commits from James-Frowen/SimpleWebTransport PR#22 into Mirror's vendored copy at Assets/Mirror/Transports/SimpleWeb/SimpleWeb/.

Background

Mirror vendors the SimpleWebTransport source directly under Assets/Mirror/Transports/SimpleWeb/SimpleWeb/. The upstream PR#22 fixed several memory leaks, thread-safety races, and added a maxSendQueueSize feature to prevent unbounded memory growth on slow connections. All changes must be adapted to Mirror's version of the code (which uses int connId directly instead of IConnection/Connection with connId fields, and has a slightly different SimpleWebServer and WebSocketServer API).


Changes to port (one commit at a time, in order)

Commit 1 — fix: adding count == 0 check to SendAll before taking ArrayBuffer

File: Assets/Mirror/Transports/SimpleWeb/SimpleWeb/Server/SimpleWebServer.cs

In SimpleWebServer.SendAll(List<int> connectionIds, ...):

// ADD early-exit guard before taking a buffer:
if (connectionIds.Count == 0)
    return;

This is the only SendAll overload that exists in Mirror's SimpleWebServer. Add the guard.

Commit 2 — fix: simplify legacy SendAll and calling release inside legacy Send

File: Assets/Mirror/Transports/SimpleWeb/SimpleWeb/Server/WebSocketServer.cs

In WebSocketServer.Send(int id, ArrayBuffer buffer):

// BEFORE:
if (connections.TryGetValue(id, out Connection conn))
{
    conn.sendQueue.Enqueue(buffer);
    conn.sendPending.Set();
}
else
    Log.Warn("...");

// AFTER:
if (connections.TryGetValue(id, out Connection conn))
{
    conn.sendQueue.Enqueue(buffer);
    conn.sendPending.Set();
}
else
{
    Log.Warn("[SWT-WebSocketServer]: Send: cannot send message to {0} because it was not found in dictionary. Maybe it disconnected.", id);
    buffer.Release(); // ADD: release buffer if connection not found
}

Commit 3 — fix: using auto dispose to safely release buffers

File: Assets/Mirror/Transports/SimpleWeb/SimpleWeb/Client/SimpleWebClient.cs

In ProcessMessageQueue, change the EventType.Data case from manual release to using:

// BEFORE:
case EventType.Data:
    onData?.Invoke(next.data.ToSegment());
    next.data.Release();
    break;

// AFTER:
case EventType.Data:
    using (next.data) // auto release
    {
        onData?.Invoke(next.data.ToSegment());
    }
    break;

File: Assets/Mirror/Transports/SimpleWeb/SimpleWeb/Server/SimpleWebServer.cs

In ProcessMessageQueue(MonoBehaviour behaviour), same change:

// BEFORE:
case EventType.Data:
    onData?.Invoke(next.connId, next.data.ToSegment());
    next.data.Release();
    break;

// AFTER:
case EventType.Data:
    using (next.data)
    {
        onData?.Invoke(next.connId, next.data.ToSegment());
    }
    break;

File: Assets/Mirror/Transports/SimpleWeb/SimpleWeb/Common/SendLoop.cs

Replace manual msg.Release() calls with using ArrayBuffer _ = msg; pattern in both the batchSend and non-batch branches. In the batchSend branch:

// BEFORE:
while (conn.sendQueue.TryDequeue(out ArrayBuffer msg))
{
    if (!client.Connected)
    {
        Log.Verbose(...);
        msg.Release();
        return;
    }
    ...
    offset = SendMessage(writeBuffer, offset, msg, setMask, maskHelper);
    msg.Release();
}

// AFTER:
while (conn.sendQueue.TryDequeue(out ArrayBuffer msg))
{
    using ArrayBuffer _ = msg; // auto release
    if (!client.Connected)
    {
        Log.Verbose(...);
        return;
    }
    ...
    offset = SendMessage(writeBuffer, offset, msg, setMask, maskHelper);
}

Same pattern in the non-batch branch (remove both msg.Release() calls and add using ArrayBuffer _ = msg; at the top of the loop body).

Commit 4 — fix: adding thread safe send method to ensure buffer is released

File: Assets/Mirror/Transports/SimpleWeb/SimpleWeb/Common/Connection.cs

The current Mirror Connection has sendPending and sendQueue as public fields. The upstream change makes them private and gates access behind a new QueueSend method and a GetSendQueue accessor. However, Mirror's Connection does NOT have onSendQueueFull or maxSendQueueSize (those come in commit 6). Apply the subset that's needed here:

  1. Add SetNeedsPong() method (replaces direct field access in ReceiveLoop):
public void SetNeedsPong()
{
    needsPong = true;
    sendPending.Set();
}
  1. Add QueueSend(ArrayBuffer buffer) method with the dispose-safe lock:
public void QueueSend(ArrayBuffer buffer)
{
    lock (disposedLock)
    {
        if (hasDisposed)
        {
            Log.Warn($"Message sent to id={connId} after it was been disposed");
            buffer.Release();
        }
        else
        {
            sendQueue.Enque...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

<!-- START COPILOT CODING AGENT TIPS -->
---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs.

Copilot AI and others added 2 commits March 26, 2026 08:45
…ream PR#22

Co-authored-by: MrGadget1024 <9826063+MrGadget1024@users.noreply.github.com>
Agent-Logs-Url: https://github.com/MirrorNetworking/Mirror/sessions/32167aa1-1cc0-4815-a73f-cb57882b4de6
…ompatibility

Co-authored-by: MrGadget1024 <9826063+MrGadget1024@users.noreply.github.com>
Agent-Logs-Url: https://github.com/MirrorNetworking/Mirror/sessions/3a1b9d47-a0ec-407b-83d3-b2ac7b1189c3
Copilot AI changed the title [WIP] Fix Queue<T>.TryDequeue not available before Unity 2022.3 fix(SimpleWebTransport): Guard Queue.TryDequeue behind UNITY_2022_3_OR_NEWER Mar 26, 2026
Copilot AI requested a review from MrGadget1024 March 26, 2026 08:48
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 26, 2026

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.56%. Comparing base (df4fed3) to head (9d54312).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
.../Mirror/Transports/SimpleWeb/SimpleWebTransport.cs 0.00% 2 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4105      +/-   ##
==========================================
- Coverage   43.56%   43.56%   -0.01%     
==========================================
  Files         150      150              
  Lines       14626    14627       +1     
==========================================
  Hits         6372     6372              
- Misses       8254     8255       +1     
Flag Coverage Δ
unittests 43.56% <0.00%> (-0.01%) ⬇️
unity-6000.4.0f1 43.56% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../Mirror/Transports/SimpleWeb/SimpleWebTransport.cs 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MrGadget1024 MrGadget1024 marked this pull request as ready for review March 26, 2026 09:30
@miwarnec miwarnec merged commit d569f5e into master Mar 26, 2026
9 of 10 checks passed
@miwarnec miwarnec deleted the copilot/fix-queue-trydequeue-compatibility branch March 26, 2026 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants