CNA-BUG-029: Copying a GraphicsResource bypasses GraphicsDevice resource tracking (copies of Texture2D and TextureCube escape device disposal)

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

A copy-constructed Texture2D or TextureCube is never registered with its GraphicsDevice, so device disposal neither disposes it nor releases the renderer texture it shares, and a cross-device copy assignment leaves a stale registration behind.

Identifier

CNA-BUG-029

Category

Bug

Subsystem

Graphics & renderers

Status

Narrowed (partially fixed; describes only what survives)

Verified against

CNA 009d40f5 (009d40f5dd085c4e674d3479675fac84b12b3e0a)

Severity

Medium (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::Graphics::GraphicsResource copy constructor and copy assignment; Texture2D(const Texture2D&), TextureCube(const TextureCube&); GraphicsDevice::AddResourceReference / GraphicsDevice::Dispose()

Expected behaviour

GraphicsDevice::AddResourceReference is documented as the registration that lets the device “dispose registered resources before its own renderer is destroyed, preventing use-after-free”. Every live wrapper that can keep a renderer allocation alive should therefore be known to that device: a copy should be registered (or share the source's registration), be disposed by GraphicsDevice::Dispose() with the original, and a copy assignment that changes a wrapper's device should move the registration, exactly as the move operations already do.

Actual behaviour at TARGET

• Fixed since the finding was first recorded: moves. GraphicsResource(GraphicsResource&&) and operator=(GraphicsResource&&) call GraphicsDevice::TransferResourceReference, texture moves also call TransferMovedTexture, and the GraphicsResource-level device calls are gated on the weak graphicsDeviceLifetime_ token, so a moved-from address no longer stays in the registry and GraphicsResource::Dispose(bool) no longer calls into a destroyed device.
• Still present: copies. GraphicsResource(const GraphicsResource&) copies the device pointer, the lifetime token, name and tag but never calls AddResourceReference. Texture2D and TextureCube default their copy operations, so the copy also shares the original's std::shared_ptr<ITextureRenderer>. Consequences, all by reading:
• GetTrackedResourceCount() does not count the copy, and GraphicsDevice::Dispose() disposes only the registered original. The copy keeps reporting getIsDisposedProperty() == false and keeps the renderer texture alive; the allocation is released when the last copy dies, which can be after the renderer and its native context are gone (for example a static or global Texture2D destroyed at process exit). Whether that late release is harmless is backend- and driver-specific: EasyGLTextureRenderer's destructor, for instance, still deletes its GL name through its texture member once its registry has expired.
• A copy that outlives its device also reaches Texture::Dispose (through Texture2D::Dispose or TextureCube::Dispose) with isDisposed_ still false, and Texture::Dispose calls RemoveDisposedTexture on graphicsDevice_->getTexturesProperty() and getVertexTexturesProperty() without checking the lifetime token, so it reads (and, if a slot still names the texture, writes) the storage of a GraphicsDevice that no longer exists. Registered originals do not reach that call, because GraphicsDevice::Dispose() has already marked them disposed.
• GraphicsResource::operator=(const GraphicsResource&) re-points graphicsDevice_ without removing the wrapper from its old device or adding it to the new one. A Texture2D constructed on device A and then copy-assigned from a texture of device B stays in A's registry; if it is destroyed first, its Dispose(false) unregisters from B (a no-op) and leaves a dangling pointer in A's registry, which A.Dispose() later calls Dispose() through.
• These copies are routine, not exotic: ContentManager::Load<T> stores a copy of every loaded Texture2D in its std::any cache and returns a further copy on every cache hit, so only the wrapper returned by the first load of an asset is registered.

Source locations

• modules/graphics/src/Xna/GraphicsResource.cpp — GraphicsResource copy constructor and copy assignment: no registration change; the move constructor and move assignment call TransferResourceReference
• modules/graphics/include/Microsoft/Xna/Framework/Graphics/GraphicsResource.hpp — copy operations documented as carrying device, name and tag
• modules/graphics/src/Xna/GraphicsDevice.cpp — AddResourceReference, RemoveResourceReference, TransferResourceReference, Dispose(bool) drains only resources_
• modules/graphics/include/Microsoft/Xna/Framework/Graphics/GraphicsDevice.hpp — AddResourceReference contract comment; GetTrackedResourceCount()
• modules/graphics/include/Microsoft/Xna/Framework/Graphics/Texture2D.hpp — Texture2D copy constructor and copy assignment are defaulted; a copy shares the renderer
• modules/graphics/src/Xna/Texture2D.cpp — Texture2D move operations (TransferMovedTexture) and Dispose(bool)
• modules/graphics/include/Microsoft/Xna/Framework/Graphics/TextureCube.hpp — TextureCube copy constructor and copy assignment are defaulted
• modules/content/include/Microsoft/Xna/Framework/Content/ContentManager.hpp — Load: the cache stores a copy (loadedAssets_[cacheKey] = result) and every cache hit returns another copy through std::any_cast

Evidence

Checked by reading GraphicsResource.cpp, GraphicsDevice.cpp, Texture2D.cpp and ContentManager.hpp at 009d40f5. The move fix is older than this snapshot; earlier descriptions of a moved-from address left in the registry refer to a previous revision. Not executed: a focused run needs a GraphicsDevice with a real renderer, which is outside a pure-source probe.

Uncertain: whether any shipped backend crashes, leaks or silently succeeds when a shared texture allocation is released after its renderer; that depends on the backend and the driver and was not examined per backend. Copy-constructed state objects (BlendState, DepthStencilState, RasterizerState, SamplerState) are also unregistered, but they own no renderer allocation, so for them only the disposed flag differs.

Independently observed as a separate finding (merged): The GraphicsResource copy constructor neither registers the copy nor raises ResourceCreated, but Dispose(bool) (also run from the destructor) raises ResourceDestroyed for any live device, so every short-lived copy produces an unmatched destroy event.

Focused reproduction

Illustrative, not compiled (uses the public CNAEXT counter):

GraphicsDevice device;
const std::size_t baseline = device.GetTrackedResourceCount();
Texture2D original(device, 4, 4);          // registered: baseline + 1
{
    Texture2D copy(original);              // shares original's ITextureRenderer
    // expected baseline + 2; at 009d40f5 still baseline + 1
    assert(device.GetTrackedResourceCount() == baseline + 1);
    device.Dispose();
    assert(original.getIsDisposedProperty());
    assert(!copy.getIsDisposedProperty());  // escaped: still holds the renderer texture
}

Current tests

GraphicsResourceDisplayModeCollectionTests.cpp (CrossDeviceMoveAssignmentTransfersTrackingAndDetachesOldBindings) pins move-assignment tracking across two devices for vertex buffers, index buffers and textures. Texture2DTests.cpp copy tests use device-less textures and check only dimensions and format; TextureCubeTests.cpp checks that a copy shares the renderer. ContentManagerTextureCacheCycleTests.cpp covers cache teardown only in the safe order (cache and wrappers before the device). Nothing checks the tracked count after a copy, the disposed state of a copy after device disposal, or a copy assignment across devices.

Regression test

Add to GraphicsResourceTest: (1) a copy of a device-created Texture2D raises GetTrackedResourceCount() by one and is disposed by GraphicsDevice::Dispose(); (2) the cross-device case of the existing move test, done with copy assignment and the destination destroyed before the old device, run under AddressSanitizer; (3) a ContentManager cache hit disposed with its device.

Blast radius

Affected: copyable graphics resources that own renderer allocations (Texture2D, TextureCube), in particular every wrapper handed out by a ContentManager cache hit; code that disposes a GraphicsDevice explicitly while copies live; copies that outlive the device (statics, globals, objects owned outside the Game); programs with more than one device. Not affected: move-only resources (VertexBuffer, IndexBuffer, RenderTarget2D, Texture3D), moves in general, and the ordinary Game shape, where the derived game's members and then Content_ are destroyed before GraphicsDevice_, so every copy dies while the renderer still exists.

Workaround

Keep texture wrappers inside the lifetime of the Game (members of the derived game, never statics or globals), do not copy-assign a texture onto a wrapper of another device, and prefer moving a texture to copying it.