Skip to content

Commit d555dc6

Browse files
Copilotstephentoub
andcommitted
Remove unnecessary SemaphoreSlims for _permissionHandlerLock and _userInputHandlerLock
Replace SemaphoreSlim-based locking with volatile fields for _permissionHandler and _userInputHandler. These fields are single reference types where reads and writes are atomic in .NET, so locking is unnecessary. The volatile keyword ensures proper visibility across threads. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
1 parent f556505 commit d555dc6

File tree

1 file changed

+7
-51
lines changed

1 file changed

+7
-51
lines changed

dotnet/src/Session.cs

Lines changed: 7 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,8 @@ public partial class CopilotSession : IAsyncDisposable
4747
private readonly HashSet<SessionEventHandler> _eventHandlers = new();
4848
private readonly Dictionary<string, AIFunction> _toolHandlers = new();
4949
private readonly JsonRpc _rpc;
50-
private PermissionRequestHandler? _permissionHandler;
51-
private readonly SemaphoreSlim _permissionHandlerLock = new(1, 1);
52-
private UserInputHandler? _userInputHandler;
53-
private readonly SemaphoreSlim _userInputHandlerLock = new(1, 1);
50+
private volatile PermissionRequestHandler? _permissionHandler;
51+
private volatile UserInputHandler? _userInputHandler;
5452
private SessionHooks? _hooks;
5553
private readonly SemaphoreSlim _hooksLock = new(1, 1);
5654
private SessionRpc? _sessionRpc;
@@ -300,15 +298,7 @@ internal void RegisterTools(ICollection<AIFunction> tools)
300298
/// </remarks>
301299
internal void RegisterPermissionHandler(PermissionRequestHandler handler)
302300
{
303-
_permissionHandlerLock.Wait();
304-
try
305-
{
306-
_permissionHandler = handler;
307-
}
308-
finally
309-
{
310-
_permissionHandlerLock.Release();
311-
}
301+
_permissionHandler = handler;
312302
}
313303

314304
/// <summary>
@@ -318,16 +308,7 @@ internal void RegisterPermissionHandler(PermissionRequestHandler handler)
318308
/// <returns>A task that resolves with the permission decision.</returns>
319309
internal async Task<PermissionRequestResult> HandlePermissionRequestAsync(JsonElement permissionRequestData)
320310
{
321-
await _permissionHandlerLock.WaitAsync();
322-
PermissionRequestHandler? handler;
323-
try
324-
{
325-
handler = _permissionHandler;
326-
}
327-
finally
328-
{
329-
_permissionHandlerLock.Release();
330-
}
311+
var handler = _permissionHandler;
331312

332313
if (handler == null)
333314
{
@@ -354,15 +335,7 @@ internal async Task<PermissionRequestResult> HandlePermissionRequestAsync(JsonEl
354335
/// <param name="handler">The handler to invoke when user input is requested.</param>
355336
internal void RegisterUserInputHandler(UserInputHandler handler)
356337
{
357-
_userInputHandlerLock.Wait();
358-
try
359-
{
360-
_userInputHandler = handler;
361-
}
362-
finally
363-
{
364-
_userInputHandlerLock.Release();
365-
}
338+
_userInputHandler = handler;
366339
}
367340

368341
/// <summary>
@@ -372,16 +345,7 @@ internal void RegisterUserInputHandler(UserInputHandler handler)
372345
/// <returns>A task that resolves with the user's response.</returns>
373346
internal async Task<UserInputResponse> HandleUserInputRequestAsync(UserInputRequest request)
374347
{
375-
await _userInputHandlerLock.WaitAsync();
376-
UserInputHandler? handler;
377-
try
378-
{
379-
handler = _userInputHandler;
380-
}
381-
finally
382-
{
383-
_userInputHandlerLock.Release();
384-
}
348+
var handler = _userInputHandler;
385349

386350
if (handler == null)
387351
{
@@ -589,15 +553,7 @@ await InvokeRpcAsync<object>(
589553
_eventHandlers.Clear();
590554
_toolHandlers.Clear();
591555

592-
await _permissionHandlerLock.WaitAsync();
593-
try
594-
{
595-
_permissionHandler = null;
596-
}
597-
finally
598-
{
599-
_permissionHandlerLock.Release();
600-
}
556+
_permissionHandler = null;
601557
}
602558

603559
private class OnDisposeCall(Action callback) : IDisposable

0 commit comments

Comments
 (0)