Skip to content

Use new Lock & make internal + private classes sealed to avoid virtual dispatch - #998

Merged
KevinJump merged 1 commit into
KevinJump:v17/mainfrom
Henr1k80:v17/sealed
Aug 5, 2026
Merged

Use new Lock & make internal + private classes sealed to avoid virtual dispatch#998
KevinJump merged 1 commit into
KevinJump:v17/mainfrom
Henr1k80:v17/sealed

Conversation

@Henr1k80

Copy link
Copy Markdown
Contributor

No description provided.

@KevinJump
KevinJump merged commit 3109ab6 into KevinJump:v17/main Aug 5, 2026
4 checks passed
KevinJump added a commit that referenced this pull request Aug 5, 2026
* Avoid some unneeded dictionary operations (#997)

(cherry picked from commit 1d06d7c)

* Use new Lock & make internal + private classes sealed to avoid virtual dispatch (#998)

(cherry picked from commit 3109ab6)

* Changelog for the v17 perf ports (#997, #998)

Records the SyncChangeInfo sealing, which is a (minor) extender API change:
it stays protected so handlers can still construct one from an
IsItemCurrentAsync override, but can no longer be derived from.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Henrik <hg@impact.dk>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
KevinJump added a commit that referenced this pull request Aug 5, 2026
…1020)

ClaimItemForExport locked on notification.State - Umbraco's dictionary,
not ours. Anything else holding a reference to it could contend with us on
an object neither side knows the other is using. Replaced with a private
static Lock, following the pattern from #998.

Statics on a generic type are per closed type, so documents and elements
get a lock each. That suits us: they never share a notification state, so
they have nothing to contend over.

Added a test that races the claim across threads. Verified it fails without
the lock - concurrent HashSet.Add corrupts the set and throws
IndexOutOfRangeException - so it actually proves the guard is doing
something.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants