Skip to content

Make NGitLab.Mock.Clients.ClientContext lock async-friendly #910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

louis-z
Copy link
Member

@louis-z louis-z commented Jun 17, 2025

The locking mechanism was reported as throwing a System.Threading.SynchronizationLockException when used in an async context. This is because continuation of an awaited Task may occur on a different thread than the initial one, which means an attempt to release the lock from a different thread than the one that "owns" it.

  • Add NGitLab.Mock.Tests.FileTests to reproduce the issue
  • Implement the solution from Make NGitLab.Mock.Clients.ClientContext usable in an async context #902, but this causes another issue when all tests are run: a deadlock!
  • But wait, is that locking scheme even required? Let's remove it and see what happens...
  • Determine where reentrant locks are attempted and adapt code so we use "lockless" logic within locked scopes

@louis-z louis-z marked this pull request as ready for review June 17, 2025 19:38
@louis-z louis-z requested a review from a team as a code owner June 17, 2025 19:56
@louis-z louis-z requested review from Toa741 and removed request for a team June 17, 2025 19:56
@louis-z louis-z marked this pull request as draft June 17, 2025 20:01
@louis-z louis-z removed the request for review from Toa741 June 17, 2025 20:02
@louis-z louis-z force-pushed the remove-lock-from-mocks-clientcontext branch 2 times, most recently from 7ca8a1f to da09093 Compare June 27, 2025 12:26
@louis-z louis-z changed the title Remove lock mechanism from NGitLab.Mock.Clients.ClientContext Make NGitLab.Mock.Clients.ClientContext lock async-friendly Jun 27, 2025
@louis-z louis-z marked this pull request as ready for review June 27, 2025 12:35
@louis-z louis-z requested a review from Toa741 June 27, 2025 12:36
@louis-z louis-z force-pushed the remove-lock-from-mocks-clientcontext branch 2 times, most recently from 1dc2da2 to 7a054ea Compare July 7, 2025 13:13
ap0llo and others added 2 commits July 15, 2025 14:33
That lock mechanism was not needed and caused issues when using the
ClientContext in an async context.
@louis-z louis-z force-pushed the remove-lock-from-mocks-clientcontext branch from 7a054ea to e4f21fb Compare July 15, 2025 18:33
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