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

CNA snapshot 3b43bd86  ·  Known Issues › Current bugs  ·  source links pinned to 3b43bd86

The XNA stock effects, SpriteEffect, the compiled Effect::Clone() and, since CNA-DEMOS-ISSUE-055, ShaderEffect refuse a disposed source, but the CNA extension effects PbrEffect, SkinnedPbrEffect and ColorMatrixEffect copy-construct through Effect(device) and do not check.

Identifier

CNA-BUG-106

Category

Bug

Subsystem

Graphics & renderers

Status

Narrowed (partially fixed; describes only what survives)

Verified against

CNA 3b43bd86 (3b43bd8687ed46523d873f9bec4b32bd8481dcb5)

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

Effect::Clone() on CNA PbrEffect, SkinnedPbrEffect and ColorMatrixEffect

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

PbrEffect::Clone, SkinnedPbrEffect::Clone and ColorMatrixEffect::Clone (PbrEffect.cpp, SkinnedPbrEffect.cpp, ColorMatrixEffect.cpp) 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.

Fixed since first recorded: ShaderEffect::Clone (ShaderEffect.cpp) now calls ThrowIfDisposedForCloneInternal() first (1ba26ed18, CNA-DEMOS-ISSUE-055).

Source locations

• modules/graphics/src/Xna/PbrEffect.cpp — PbrEffect copy constructor through Effect(device) and PbrEffect::Clone: no disposed check
• modules/graphics/src/Xna/SkinnedPbrEffect.cpp — SkinnedPbrEffect copy constructor and Clone: no disposed check
• modules/graphics/src/Xna/ColorMatrixEffect.cpp — ColorMatrixEffect copy constructor and Clone: no disposed check
• modules/graphics/src/Xna/Effect.cpp — Effect::ThrowIfDisposedForCloneInternal
• modules/graphics/src/Xna/ShaderEffect.cpp — ShaderEffect::Clone: refuses a disposed source since 1ba26ed18

Evidence

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

Independent re-verification: Checked by reading every Effect subclass's Clone and copy constructor at 3b43bd86; nothing was built or executed.

Re-verified at c6dec91f: 1ba26ed18 added the check to ShaderEffect::Clone with ShaderCloneStateTest.DisposedSourceIsRefusedWithExactException; the three extension effects' copy constructors are unchanged. Not executed.

Focused reproduction

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

Current tests

EffectTests.cpp (StockEffectClonesRejectDisposedSources, CloneAfterDisposeThrowsObjectDisposedException), StockEffectCloneConstructorTest.CloneOfADisposedEffectIsRefused and ShaderCloneStateTest.DisposedSourceIsRefusedWithExactException cover the XNA stock effects, SpriteEffect, EffectMaterial, compiled effects and ShaderEffect. None clones a disposed PbrEffect, SkinnedPbrEffect or ColorMatrixEffect.

Regression test

The same disposed-clone assertion for PbrEffect, SkinnedPbrEffect and ColorMatrixEffect.

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.