Skip to content

Conversation

@AndyButland
Copy link
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

Resolves: #19338

Description

The linked issue reports a regression after #17246, which was introduced to avoid locking exceptions in asynchronous code.

It seems that after this, near-simultaneous requests to save and publish content results in one or more of the requests failing due to NuCache being unable to acquire a lock (the SemaphoneSlim that's it's based on not being released from the previous operation). You would then find the content created and published, but the failed ones wouldn't be in the cache, which is manifested in the backoffice by the message of "published but not in the cache" shown where the URLs are displayed on the "Info" panel.

To resolve I've removed the "fast-fail" check we had before waiting for the SemaphoneSlim, and allowed the wait to pick up the lock when released from the previous operation.

Some advice from @ronaldbarendse:

The recursive lock check was probably added as a defence to fail-fast, because a Monitor is shared on the current thread and could therefore take more than 1 write lock. However, I don't believe the SemaphoreSlim is susceptible to the same problem.

Testing

The original issue can be replicated as shown in the linked issue from the browser console when logged into the backoffice.

You should find with this PR in place multiple requests to create, save and publish content will succeed.

…tion.

Added additional check to ensure we don't release a lock that's already released.
Copilot AI review requested due to automatic review settings May 27, 2025 13:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the fast-fail recursive lock check on the NuCache write semaphore and refines the release logic to avoid lock acquisition exceptions under near-simultaneous publish requests. It also fixes a typo in the exception message.

  • Corrected typo in EnsureLocked exception text.
  • Removed the recursive-lock guard in Lock(...).
  • Updated Release(...) to conditionally release or log a warning when the semaphore count is unexpected.
Comments suppressed due to low confidence (2)

src/Umbraco.PublishedCache.NuCache/ContentStore.cs:409

  • Add unit or integration tests for concurrent lock release scenarios, especially cases where CurrentCount is non-zero, to verify that the semaphore is released correctly and no deadlocks occur.
private void Release(WriteLockInfo lockInfo, bool commit = true)

src/Umbraco.PublishedCache.NuCache/ContentStore.cs:418

  • [nitpick] Consider enhancing this warning to include more context (e.g., operation name or lockInfo details) so that it's easier to trace why an unexpected count occurred in production logs.
"On releasing the content store lock the current count was found unexpectedly to be non-zero with a value of {CurrentCount}."

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.

4 participants