fix: WearableStorage locks on the shared base-registry monitor

[eordano]

Identity swap fully unequips

Production evidence (decentraland Sentry, archive snapshot 2026-07-17):

• UNITY-EXPLORER-PF3: 17 events / 1 users, last seen 2026-07-06
• UNITY-EXPLORER-KW0: 5 events / 2 users, last seen 2026-07-13
• UNITY-TEST-ENVIRONMENT-TT: 4 events / 2 users, last seen 2026-03-26
• UNITY-EXPLORER-NJ1: 3 events / 3 users, last seen 2026-04-19
• UNITY-EXPLORER-NJ0: 3 events / 3 users, last seen 2026-03-17
• UNITY-EXPLORER-MBC: 3 events / 1 users, last seen 2026-02-16

fixes #9304
fixes #9303
fixes #7397
fixes #7396

---

Supersedes #9410: moved from the fork into the org repo so CI workflows receive repository secrets (fork PRs do not).

[github-actions]

New build in progress, come back later!

[github-actions]

Slack notification sent to #explorer-ext-contributions for external review.
To re-send, delete this comment and re-add the ext-contribution label.

[eordano]

Closed in favor of the compound PR #9430 (team decision — all fixes in this set land and iterate there; each fix remains individually reviewable as its own merge commit in #9430).

[decentraland-bot]

PR Review: fix: WearableStorage locks on the shared base-registry monitor

STEP 2 — Root-cause check: PASS ✅

The PR fixes two distinct root causes confirmed by Sentry:

1. KeyNotFoundException on identity swap (#9304 upper_body, #9303 body_shape): Equip() can add non-priority category keys (categories outside CATEGORIES_PRIORITY). The old UnEquipAll() only nulled priority-category entries, leaving stale non-priority keys. After an identity change, lookups for standard categories that happened to not be in the dictionary threw KeyNotFoundException via the direct wearables[category] indexer.
2. Shadowed lockObject in WearableStorage: The derived class declared its own private readonly object lockObject, shadowing the base class AvatarElementNftRegistry.lockObject (protected). Result: lock(lockObject) in WearableStorage methods and lock(lockObject) in AvatarElementNftRegistry methods used different monitors — concurrent callers could interleave unsafely across base/derived boundaries.

Both fixes address causes, not symptoms.

STEP 3 — Design & integration: PASS ✅

No new units introduced. All changes modify existing methods in EquippedWearables and remove dead/shadowing fields from WearableStorage.

• UnEquipAll() Clear+re-seed: Correct approach. The dictionary must (a) lose all non-priority keys from the previous identity and (b) retain seeded priority keys with null values for Items() consumers. Clear+Add achieves both.
• Wearable() → TryGetValue: Aligns the implementation with the interface contract (IWearable? Wearable(string) — nullable return). The old direct indexer violated this by throwing instead of returning null.
• Clear() → UnEquipAll(): Maintains the seeding invariant. The old bare wearables.Clear() left the dictionary empty, so subsequent Wearable() calls for priority categories would find no key.
• Removed ownedNftsRegistry: Dead field — shadowed AvatarElementNftRegistry.ownedNftRegistry and was never referenced by any code in WearableStorage.
• Removed lockObject: Fixes the shadowing bug. All lock() calls in WearableStorage now use the base class's protected lockObject, synchronizing with SetOwnedNft, TryGetOwnedNftRegistry, etc.

STEP 4 — Member audit

Wearable(string category) — 4 external callers found:

• ElementProviderHelper.cs:50 — equippedWearables.Wearable(BODY_SHAPE)!.GetUrn() (null-forgiving !)
• ProfileExtensions.cs:38 — equippedWearables.Wearable(BODY_SHAPE)!.GetUrn() (null-forgiving !)
• HideCategoriesController.cs — properly handles nullable
• BackpackBusController.cs — properly handles nullable

The ! usages in ElementProviderHelper and ProfileExtensions are pre-existing — they assumed BODY_SHAPE is always seeded, which remains true after this fix (priority categories are always re-seeded). Not a regression.

IsEquipped(ITrimmedWearable) — now routes through Wearable() instead of the direct indexer. Correct — single code path for dictionary access.

STEP 5 — Line-level review

One P2 finding posted as an inline comment below.

Security review: No security issues. The locking fix improves thread safety. No secrets, no input validation concerns, no auth changes.

STEP 6 — Complexity: SIMPLE

2 files, +11/−9 lines, straightforward dictionary-management fix and dead-field removal.

STEP 7 — QA: YES

Touches runtime avatar rendering code (EquippedWearables is used during identity swap, backpack, profile updates). Identity swap and wearable equip/unequip flows should be tested.

STEP 8 — Non-blocking warnings

None.

---

REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: Dictionary-management fix in EquippedWearables and dead-field removal in WearableStorage — no ECS, async, or architectural changes.
QA_REQUIRED: YES

---

Reviewed by Jarvis 🤖 · Requested by eordano via Slack

[decentraland-bot]

[P2] Comment narrates caller/external behavior (CLAUDE.md §11 "Comments that narrate caller/external behavior"). The WHY (Clear+re-seed vs. bare null-assignment) is genuinely non-obvious and worth documenting, but the current 4-line comment describes what Equip() does and what callers expect — behavior that can change without UnEquipAll() changing. Trim to the invariant itself.

Suggested change

	// Re-seed instead of a bare Dictionary.Clear(): reads rely on every seeded category key
	// surviving with a null value (Clear() made category reads throw KeyNotFoundException after
	// an identity change), and Equip() can add non-priority category keys that must not keep
	// the previous identity's wearables equipped.
	// Clear + re-seed: bare null-assignment leaves non-priority keys from a prior Equip();
	// bare Clear() drops the seeded keys that reads and Items() depend on.