Content Publishing: Fix deadlocks by acquiring WriteLock at outer scope

[lauraneto]

Summary

This PR fixes SQL Server deadlocks during concurrent content publishing by acquiring the WriteLock(Constants.Locks.ContentTree) at the outer scope in ContentPublishingService.PublishAsync() rather than letting nested services acquire locks independently, which can cause deadlock cycles.

This issue was discovered while load testing #21102 (re-introduce lazy locks).

Example Deadlock Exception

Without this fix, concurrent publish operations frequently fail with:

Transaction (Process ID 63) was deadlocked on lock resources with another process 
and has been chosen as the deadlock victim. Rerun the transaction.

Stack trace shows the deadlock occurs in ContentService.Publish() when trying to acquire the WriteLock:

at SqlServerDistributedLockingMechanism.SqlServerDistributedLock.ObtainWriteLock()
at LockingMechanism.EagerWriteLockInner(...)
at ContentService.Publish(...)
at ContentPublishingService.PublishAsync(...)

Test Results (Publish-Only Load Test)

The test creates all content during setup, then measures only concurrent publish operations to isolate the specific deadlock scenario.

SQL Server (5 runs each)

WITHOUT Fix (Deadlocks Occur)

Run	HTTP Failure Rate
1	38.83%
2	38.34%
3	37.86%
4	39.80%
5	38.34%
Average	38.63%

WITH Fix (No Deadlocks)

Run	HTTP Failure Rate
1	0.00%
2	0.00%
3	0.48%
4	0.48%
5	0.00%
Average	0.19%

SQLite (5 runs each)

WITHOUT Fix

Run	HTTP Failure Rate
1	15.04%
2	8.73%
3	12.13%
4	8.73%
5	5.82%
Average	10.09%

WITH Fix

Run	HTTP Failure Rate
1	0.00%
2	0.00%
3	0.00%
4	0.00%
5	0.00%
Average	0.00%

Summary

Database	Without Fix	With Fix	Improvement
SQL Server	38.63% failures	0.19% failures	99.5% reduction
SQLite	10.09% failures	0.00% failures	100% reduction

The Fix

The change adds scope.WriteLock(Constants.Locks.ContentTree) at the entry point of ContentPublishingService.PublishAsync():

using ICoreScope scope = _coreScopeProvider.CreateCoreScope();
scope.WriteLock(Constants.Locks.ContentTree);  // <-- Prevents deadlocks
IContent? content = _contentService.GetById(key);

This ensures the content tree lock is acquired immediately in a consistent order, preventing the deadlock cycles that occur when concurrent requests try to acquire the same locks in different orders through nested service calls.

Test Methodology

• Load test: k6 with 20 concurrent VUs, 100 iterations
• Test type: Publish-only (content created during setup phase)
• Databases: SQL Server (containerized), SQLite
• Environment: macOS, Release build, .NET 10.0
• Runs per configuration: 5

Testing the Fix

A test script is attached to verify the deadlock fix. It creates content items and publishes them in parallel to reproduce the deadlock scenario.

Requirements:

• .NET 10+
• An Umbraco instance running with SQL Server
• An API User configured with client credentials (docs)

Usage:

1. Download test-concurrent-publish.cs
2. Update the configuration variables at the top of the file (URL, client ID, client secret)
3. Run: dotnet run test-concurrent-publish.cs

Expected results:

• Without the fix: Multiple publish failures due to SQL Server deadlocks
• With the fix: All publish operations succeed

[Copilot]

Pull request overview

This PR addresses SQL Server deadlocks during concurrent content publishing by acquiring the WriteLock(Constants.Locks.ContentTree) at the outer scope in ContentPublishingService.PublishAsync(). The fix prevents deadlock cycles that occur when multiple concurrent requests acquire locks in different orders through nested service calls.

Key Changes

• Added scope.WriteLock(Constants.Locks.ContentTree) immediately after creating the scope in the private PublishAsync method
• This ensures the content tree lock is acquired at the outermost scope level before calling nested ContentService methods that create their own scopes with locks
• Load testing demonstrates a 99.5% reduction in failures on SQL Server (from 38.63% to 0.19%) and 100% elimination of failures on SQLite (from 10.09% to 0%)

[AndyButland]

A one-liner but a big improvement for these contentious scenarios. I've used your script to replicate your tests and see a big improvement:

Before - Results: 25 success, 75 failed
After  - Results: 100 success, 0 failed

Great catch and thanks in particular for the clear testing steps - can see a script like this will be useful in other scenarios.