Fix shellmap OrderManager use after dispose.

This commit is contained in:
Paul Chote
2021-08-25 12:38:10 +01:00
committed by abcdefg30
parent 8f412f869d
commit e0e219793f
4 changed files with 36 additions and 18 deletions

View File

@@ -84,7 +84,14 @@ namespace OpenRA
static void JoinInner(OrderManager om) static void JoinInner(OrderManager om)
{ {
// HACK: The shellmap World and OrderManager are owned by the main menu's WorldRenderer instead of Game.
// This allows us to switch Game.OrderManager from the shellmap to the new network connection when joining
// a lobby, while keeping the OrderManager that runs the shellmap intact.
// A matching check in World.Dispose (which is called by WorldRenderer.Dispose) makes sure that we dispose
// the shellmap's OM when a lobby game actually starts.
if (OrderManager?.World == null || OrderManager.World.Type != WorldType.Shellmap)
OrderManager?.Dispose(); OrderManager?.Dispose();
OrderManager = om; OrderManager = om;
} }

View File

@@ -45,6 +45,7 @@ namespace OpenRA.Network
readonly Queue<(int Frame, int SyncHash, ulong DefeatState)> sync = new Queue<(int, int, ulong)>(); readonly Queue<(int Frame, int SyncHash, ulong DefeatState)> sync = new Queue<(int, int, ulong)>();
readonly Queue<(int Frame, OrderPacket Orders)> orders = new Queue<(int, OrderPacket)>(); readonly Queue<(int Frame, OrderPacket Orders)> orders = new Queue<(int, OrderPacket)>();
readonly Queue<OrderPacket> immediateOrders = new Queue<OrderPacket>(); readonly Queue<OrderPacket> immediateOrders = new Queue<OrderPacket>();
bool disposed;
int IConnection.LocalClientId => LocalClientId; int IConnection.LocalClientId => LocalClientId;
@@ -72,8 +73,15 @@ namespace OpenRA.Network
void IConnection.Receive(OrderManager orderManager) void IConnection.Receive(OrderManager orderManager)
{ {
while (immediateOrders.TryDequeue(out var i)) while (immediateOrders.TryDequeue(out var i))
{
orderManager.ReceiveImmediateOrders(LocalClientId, i); orderManager.ReceiveImmediateOrders(LocalClientId, i);
// An immediate order may trigger a chain of actions that disposes the OrderManager and connection.
// Bail out to avoid potential problems from acting on disposed objects.
if (disposed)
break;
}
// Project orders forward to the next frame // Project orders forward to the next frame
while (orders.TryDequeue(out var o)) while (orders.TryDequeue(out var o))
orderManager.ReceiveOrders(LocalClientId, (o.Frame + 1, o.Orders)); orderManager.ReceiveOrders(LocalClientId, (o.Frame + 1, o.Orders));
@@ -82,7 +90,10 @@ namespace OpenRA.Network
orderManager.ReceiveSync(s); orderManager.ReceiveSync(s);
} }
void IDisposable.Dispose() { } void IDisposable.Dispose()
{
disposed = true;
}
} }
public sealed class NetworkConnection : IConnection public sealed class NetworkConnection : IConnection
@@ -272,6 +283,11 @@ namespace OpenRA.Network
{ {
orderManager.ReceiveImmediateOrders(clientId, i); orderManager.ReceiveImmediateOrders(clientId, i);
Recorder?.Receive(clientId, i.Serialize(0)); Recorder?.Receive(clientId, i.Serialize(0));
// An immediate order may trigger a chain of actions that disposes the OrderManager and connection.
// Bail out to avoid potential problems from acting on disposed objects.
if (disposed)
return;
} }
while (sentSync.TryDequeue(out var s)) while (sentSync.TryDequeue(out var s))
@@ -312,6 +328,11 @@ namespace OpenRA.Network
if (record) if (record)
Recorder?.Receive(p.FromClient, p.Data); Recorder?.Receive(p.FromClient, p.Data);
// An immediate order may trigger a chain of actions that disposes the OrderManager and connection.
// Bail out to avoid potential problems from acting on disposed objects.
if (disposed)
return;
} }
} }

View File

@@ -128,19 +128,11 @@ namespace OpenRA.Network
public void ReceiveDisconnect(int clientIndex) public void ReceiveDisconnect(int clientIndex)
{ {
// HACK: The shellmap relies on ticking a disposed OM
if (disposed && World.Type != WorldType.Shellmap)
return;
pendingOrders.Remove(clientIndex); pendingOrders.Remove(clientIndex);
} }
public void ReceiveSync((int Frame, int SyncHash, ulong DefeatState) sync) public void ReceiveSync((int Frame, int SyncHash, ulong DefeatState) sync)
{ {
// HACK: The shellmap relies on ticking a disposed OM
if (disposed && World.Type != WorldType.Shellmap)
return;
if (syncForFrame.TryGetValue(sync.Frame, out var s)) if (syncForFrame.TryGetValue(sync.Frame, out var s))
{ {
if (s.SyncHash != sync.SyncHash || s.DefeatState != sync.DefeatState) if (s.SyncHash != sync.SyncHash || s.DefeatState != sync.DefeatState)
@@ -152,10 +144,6 @@ namespace OpenRA.Network
public void ReceiveImmediateOrders(int clientId, OrderPacket orders) public void ReceiveImmediateOrders(int clientId, OrderPacket orders)
{ {
// HACK: The shellmap relies on ticking a disposed OM
if (disposed && World.Type != WorldType.Shellmap)
return;
foreach (var o in orders.GetOrders(World)) foreach (var o in orders.GetOrders(World))
{ {
UnitOrders.ProcessOrder(this, World, clientId, o); UnitOrders.ProcessOrder(this, World, clientId, o);
@@ -168,10 +156,6 @@ namespace OpenRA.Network
public void ReceiveOrders(int clientId, (int Frame, OrderPacket Orders) orders) public void ReceiveOrders(int clientId, (int Frame, OrderPacket Orders) orders)
{ {
// HACK: The shellmap relies on ticking a disposed OM
if (disposed && World.Type != WorldType.Shellmap)
return;
if (pendingOrders.TryGetValue(clientId, out var queue)) if (pendingOrders.TryGetValue(clientId, out var queue))
queue.Enqueue((orders.Frame, orders.Orders)); queue.Enqueue((orders.Frame, orders.Orders));
else else

View File

@@ -574,6 +574,12 @@ namespace OpenRA
while (frameEndActions.Count != 0) while (frameEndActions.Count != 0)
frameEndActions.Dequeue()(this); frameEndActions.Dequeue()(this);
// HACK: The shellmap OrderManager is owned by its world in order to avoid
// problems with having multiple OMs active when joining a game lobby from the main menu.
// A matching check in Game.JoinInner handles OM disposal for all other cases.
if (Type == WorldType.Shellmap)
OrderManager.Dispose();
Game.FinishBenchmark(); Game.FinishBenchmark();
} }
} }