CNA-BUG-106: ShaderEffect, PbrEffect, SkinnedPbrEffect and ColorMatrixEffect clone a disposed source instead of throwing ObjectDisposedException

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

The five XNA stock effects, SpriteEffect and the compiled Effect::Clone() refuse a disposed source since SOFTWARE-259, but ShaderEffect::Clone rebuilds from its retained source strings, and the CNA extension effects PbrEffect, SkinnedPbrEffect and ColorMatrixEffect copy-construct through Effect(device), so none of the four checks.

Identifier

CNA-BUG-106

Category

Bug

Subsystem

Graphics & renderers

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

CNA ShaderEffect::Clone()

Expected behaviour

Effect::Clone()'s documented contract in Effect.hpp: "@throws System::ObjectDisposedException If this effect has been disposed", implemented by Effect::ThrowIfDisposedForCloneInternal and pinned for BasicEffect by StockEffectCloneConstructorTest.CloneOfADisposedEffectIsRefused.

Actual behaviour at TARGET

ShaderEffect.cpp's Clone is new ShaderEffect(*device_, vertSrc_, fragSrc_) plus three label copies, with no disposed check. PbrEffect::Clone, SkinnedPbrEffect::Clone and ColorMatrixEffect::Clone call their copy constructors, which initialise the base with Effect(*src.device_) rather than the Effect copy constructor that calls ThrowIfDisposedForCloneInternal, and add no check of their own; a disposed source is cloned silently.

Source locations

• modules/graphics/src/Xna/ShaderEffect.cpp — ShaderEffect::Clone
• modules/graphics/src/Xna/Effect.cpp — Effect::ThrowIfDisposedForCloneInternal
• modules/graphics/tests/Microsoft/Xna/Framework/Graphics/StockEffectCloneConstructorTests.cpp — CloneOfADisposedEffectIsRefused

Evidence

Checked by reading the named sources at 009d40f5; nothing was built or executed for this entry.

Independent re-verification: Checked by reading every Effect subclass's Clone and copy constructor at 009d40f5; nothing was built or executed. The disposed-clone test covers BasicEffect only.

Focused reproduction

No focused reproduction is known. Nothing has been invented here; the evidence above is what exists.

Current tests

The disposed-clone test covers BasicEffect only.

Regression test

The same test for ShaderEffect.

Blast radius

Code that clones effects from a pool that may contain disposed entries; the clone silently works instead of failing.

Workaround

No workaround is known.