CNA-BUG-082: GraphicsDeviceManager leaves dangling references when its lifetime is not nested inside its Game

CNA snapshot 009d40f5  ·  Known Issues › Current bugs  ·  source links pinned to 009d40f5

Game never clears its cached manager and service pointers, the manager never removes its ClientSizeChanged handler, and ~GraphicsDeviceManager dereferences its Game, so a manager that dies before or after its game is a use-after-free.

Identifier

CNA-BUG-082

Category

Bug

Subsystem

Core & runtime

Status

Open

Verified against

CNA 009d40f5 (009d40f5dd085c4e674d3479675fac84b12b3e0a)

Severity

Low (a triage suggestion, not a project priority)

Evidence basis

Source-verified: read at TARGET, not executed

Tests touching this area

Yes: see Current tests

Affected contract

Microsoft::Xna::Framework::GraphicsDeviceManager(Game*), ~GraphicsDeviceManager(), GraphicsDeviceManager::Dispose(); Game::getGraphicsDeviceProperty(), Game::BeginDraw()/EndDraw(), Game::Dispose()

Expected behaviour

Destroying or disposing a GraphicsDeviceManager should leave its game in a consistent state: the services it registered gone and no longer used by the game, its window subscription removed, and its destructor safe regardless of whether the game still exists. The header describes the argument only as “the game that owns this manager” and states no lifetime rule.

Actual behaviour at TARGET

Three one-way references survive: (1) registerServices subscribes a lambda capturing this to Window.ClientSizeChanged without keeping a token, and nothing removes it; (2) unregisterServices removes the two service entries, but Game keeps graphicsDeviceService_ and graphicsDeviceManager_, raw pointers cached in getGraphicsDeviceProperty, Initialize and DoInitialize and used by BeginDraw, EndDraw and Dispose(bool); (3) ~GraphicsDeviceManager calls Dispose(false), whose unregisterServices dereferences game_. A manager destroyed while its game keeps running is therefore used after free on the next frame or resize, and a manager that outlives an undisposed game reads the destroyed game in its destructor. CNA's C API states the same finding (“a canonical defect rather than anything this ABI can validate away”) and works around it by keeping every released manager alive until its game is destroyed.

Source locations

• modules/runtime/src/GraphicsDeviceManager.cpp — registerServices (ClientSizeChanged lambda, no token), unregisterServices, Dispose(bool), ~GraphicsDeviceManager
• modules/runtime/src/Game.cpp — graphicsDeviceService_/graphicsDeviceManager_ caching in getGraphicsDeviceProperty, Initialize, DoInitialize; use in BeginDraw, EndDraw, Dispose(bool)
• modules/runtime/include/Microsoft/Xna/Framework/GraphicsDeviceManager.hpp — constructor documentation (no lifetime rule)
• modules/c-api/src/CnaCApiGraphicsDeviceManager.cpp — RetiredManagers comment and workaround

Evidence

Checked by reading GraphicsDeviceManager.cpp and Game.cpp at 009d40f5; corroborated by the comment and retirement workaround in CnaCApiGraphicsDeviceManager.cpp. Not executed; no sanitizer run was made.

Independently observed as a separate finding (merged): registerServices discards the subscription token and Dispose removes only the two services, so a manager destroyed or replaced while its game window lives leaves a dangling resize callback that the next resize invokes.

Focused reproduction

Illustrative, not compiled:

MyGame game;
auto graphics = std::make_unique<GraphicsDeviceManager>(&game);
game.RunOneFrame();     // DoInitialize caches the manager as graphicsDeviceManager_
graphics.reset();       // unregisters the services; the game's cached pointers remain
game.RunOneFrame();     // BeginDraw() calls through the freed manager

Current tests

GraphicsDeviceManagerTests.cpp and GameTests.cpp construct the manager after the game on the stack (the safe nested shape) and cover repeated disposal. No test destroys a manager before a running game or lets one outlive its game.

Regression test

Two AddressSanitizer tests: destroy a manager between two RunOneFrame() calls (expect the second frame to present through the game's own device), and destroy the game before a heap-allocated manager (expect no access to the game). Both need Game to drop its cached pointers when the services are removed and the manager to keep and remove its window-event token.

Blast radius

C++ code that allocates the manager separately from the game and destroys it early or late, replaces the manager at run time, or disposes the game after the manager is gone; bindings that give the manager its own handle (the C API already works around it). The idiomatic shape, a manager that is a member of the derived game or a local declared after the game, is safe.

Workaround

Create exactly one manager per game, as a member of the derived game (or a local declared after it), and never destroy or replace it while the game lives.