diff --git a/src/dotnet/skills/refactoring-to-async/SKILL.md b/src/dotnet/skills/refactoring-to-async/SKILL.md new file mode 100644 index 0000000000..9699d92fad --- /dev/null +++ b/src/dotnet/skills/refactoring-to-async/SKILL.md @@ -0,0 +1,186 @@ +--- +name: refactoring-to-async +description: Convert synchronous .NET code to async/await, including proper Task propagation, cancellation support, and avoiding common async anti-patterns. Use when converting blocking I/O calls to async, fixing thread pool starvation, or modernizing sync-over-async code. +--- + +# Refactoring to Async + +## When to Use + +- Converting synchronous I/O-bound code to async/await +- Fixing thread pool starvation caused by blocking calls +- Modernizing legacy `.Result` / `.Wait()` / `.GetAwaiter().GetResult()` patterns +- Adding `CancellationToken` support to async call chains + +## When Not to Use + +- The code is CPU-bound (async won't help; consider `Parallel.For` or `Task.Run`) +- The synchronous code has no I/O operations +- The user wants to parallelize work, not make it async + +## Inputs + +| Input | Required | Description | +|-------|----------|-------------| +| Code to refactor | Yes | The synchronous methods to convert | +| Scope | No | Single method, class, or full call chain | + +## Workflow + +### Step 1: Identify blocking I/O calls + +Search for synchronous I/O patterns in the codebase: + +```bash +grep -rn "\.Result\b\|\.Wait()\|\.GetAwaiter()\.GetResult()\|ReadToEnd()\|\.Read()\|\.Write(" --include="*.cs" . +``` + +Common blocking patterns to convert: + +| Synchronous | Async Replacement | +|---|---| +| `stream.Read(buffer)` | `await stream.ReadAsync(buffer, ct)` | +| `stream.Write(data)` | `await stream.WriteAsync(data, ct)` | +| `reader.ReadToEnd()` | `await reader.ReadToEndAsync(ct)` | +| `File.ReadAllText(path)` | `await File.ReadAllTextAsync(path, ct)` | +| `File.WriteAllBytes(...)` | `await File.WriteAllBytesAsync(..., ct)` | +| `client.Send(request)` | `await client.SendAsync(request, ct)` | +| `connection.Open()` | `await connection.OpenAsync(ct)` | +| `command.ExecuteReader()` | `await command.ExecuteReaderAsync(ct)` | +| `Thread.Sleep(ms)` | `await Task.Delay(ms, ct)` | +| `task.Result` | `await task` | +| `task.Wait()` | `await task` | + +### Step 2: Convert bottom-up + +Start from the lowest-level I/O calls and work upward through the call chain. This avoids sync-over-async wrappers. + +**Before:** + +```csharp +public string GetUserData(int userId) +{ + var response = _httpClient.Send(new HttpRequestMessage(HttpMethod.Get, $"/users/{userId}")); + var body = new StreamReader(response.Content.ReadAsStream()).ReadToEnd(); + return body; +} +``` + +**After:** + +```csharp +public async Task GetUserDataAsync(int userId, CancellationToken ct = default) +{ + var response = await _httpClient.GetAsync($"/users/{userId}", ct); + response.EnsureSuccessStatusCode(); + return await response.Content.ReadAsStringAsync(ct); +} +``` + +### Step 3: Propagate async through the call chain + +Every caller of an async method must also become async. Follow the chain upward: + +```csharp +// Layer 1: Data access (already converted) +public async Task GetUserAsync(int id, CancellationToken ct) { ... } + +// Layer 2: Business logic (convert next) +public async Task GetUserProfileAsync(int id, CancellationToken ct) +{ + var user = await GetUserAsync(id, ct); + return MapToDto(user); // sync mapping is fine +} + +// Layer 3: API endpoint (convert last) +app.MapGet("/users/{id}", async (int id, CancellationToken ct, IUserService svc) => + await svc.GetUserProfileAsync(id, ct)); +``` + +### Step 4: Add CancellationToken support + +Accept `CancellationToken` as the last parameter in every async method and pass it through: + +```csharp +public async Task> GetOrdersAsync( + int userId, + CancellationToken ct = default) // Always provide a default +{ + var response = await _client.GetAsync($"/orders?user={userId}", ct); + var json = await response.Content.ReadAsStringAsync(ct); + return JsonSerializer.Deserialize>(json); +} +``` + +ASP.NET Core automatically supplies a `CancellationToken` that fires when the client disconnects. + +### Step 5: Update interfaces + +```csharp +// Before +public interface IUserRepository +{ + User GetById(int id); + List GetAll(); +} + +// After +public interface IUserRepository +{ + Task GetByIdAsync(int id, CancellationToken ct = default); + Task> GetAllAsync(CancellationToken ct = default); +} +``` + +### Step 6: Build and fix + +```bash +dotnet build +``` + +Common errors after async refactoring: + +| Error | Fix | +|---|---| +| `CS4032`: `await` in non-async method | Add `async` to the method signature and return `Task` or `Task` | +| `CS0029`: Cannot convert `Task` to `T` | Add `await` before the call | +| `CS0127`: Method returns `Task` but body returns value | Change return type to `Task` | +| `CS1998`: Async method lacks `await` | Remove `async` if no awaits are needed, or the method is genuinely sync | + +### Step 7: Verify no anti-patterns remain + +Search for remaining issues: + +```bash +grep -rn "\.Result\b\|\.Wait()\|\.GetAwaiter()\.GetResult()" --include="*.cs" . +``` + +This should return zero results in the refactored code paths. + +## Anti-Patterns to Avoid + +| Anti-Pattern | Problem | Correct Approach | +|---|---|---| +| `task.Result` or `task.Wait()` | Blocks thread, risks deadlock | `await task` | +| `async void` methods | Exceptions crash the process | `async Task` (except event handlers) | +| `Task.Run` wrapping async I/O | Wastes a thread pool thread | Call async method directly | +| Missing `ConfigureAwait(false)` in libraries | Can deadlock in UI/ASP.NET sync contexts | Add `ConfigureAwait(false)` in library code | +| Fire-and-forget without error handling | Swallows exceptions silently | `await` or use `_ = Task.Run(async () => { try... })` | + +## Validation + +- [ ] `dotnet build` compiles without errors +- [ ] No remaining `.Result`, `.Wait()`, or `.GetAwaiter().GetResult()` in converted code +- [ ] `CancellationToken` is propagated through the full call chain +- [ ] `dotnet test` passes (existing tests updated for async) +- [ ] No `async void` methods (except UI event handlers) + +## Common Pitfalls + +| Pitfall | Solution | +|---------|----------| +| Deadlock after conversion | Ensure `await` is used everywhere; no `.Result` mixed with `await` | +| Performance worse after conversion | Async adds overhead for CPU-bound work; only use for I/O | +| Forgetting to update tests | Test methods must return `Task` and use `await` | +| Breaking interface consumers | Consider keeping sync wrappers temporarily during staged migration | +| `ValueTask` vs `Task` confusion | Use `Task` by default; `ValueTask` only for hot-path methods that frequently return synchronously | diff --git a/src/dotnet/tests/refactoring-to-async/SyncService.csproj b/src/dotnet/tests/refactoring-to-async/SyncService.csproj new file mode 100644 index 0000000000..abfc583285 --- /dev/null +++ b/src/dotnet/tests/refactoring-to-async/SyncService.csproj @@ -0,0 +1,14 @@ + + + + Library + net8.0 + enable + enable + + + + + + + diff --git a/src/dotnet/tests/refactoring-to-async/UserService.cs b/src/dotnet/tests/refactoring-to-async/UserService.cs new file mode 100644 index 0000000000..c95086f117 --- /dev/null +++ b/src/dotnet/tests/refactoring-to-async/UserService.cs @@ -0,0 +1,115 @@ +using Microsoft.Data.SqlClient; + +namespace SyncService; + +public interface IUserRepository +{ + User GetById(int id); + List GetAll(); + void Save(User user); +} + +public class UserRepository : IUserRepository +{ + private readonly string _connectionString; + private readonly HttpClient _httpClient; + + public UserRepository(string connectionString, HttpClient httpClient) + { + _connectionString = connectionString; + _httpClient = httpClient; + } + + public User GetById(int id) + { + using var connection = new SqlConnection(_connectionString); + connection.Open(); + using var command = new SqlCommand("SELECT Id, Name, Email FROM Users WHERE Id = @Id", connection); + command.Parameters.AddWithValue("@Id", id); + using var reader = command.ExecuteReader(); + if (reader.Read()) + { + return new User + { + Id = reader.GetInt32(0), + Name = reader.GetString(1), + Email = reader.GetString(2) + }; + } + throw new InvalidOperationException($"User {id} not found"); + } + + public List GetAll() + { + using var connection = new SqlConnection(_connectionString); + connection.Open(); + using var command = new SqlCommand("SELECT Id, Name, Email FROM Users", connection); + using var reader = command.ExecuteReader(); + var users = new List(); + while (reader.Read()) + { + users.Add(new User + { + Id = reader.GetInt32(0), + Name = reader.GetString(1), + Email = reader.GetString(2) + }); + } + return users; + } + + public void Save(User user) + { + using var connection = new SqlConnection(_connectionString); + connection.Open(); + using var command = new SqlCommand( + "INSERT INTO Users (Name, Email) VALUES (@Name, @Email)", connection); + command.Parameters.AddWithValue("@Name", user.Name); + command.Parameters.AddWithValue("@Email", user.Email); + command.ExecuteNonQuery(); + } +} + +public class UserService +{ + private readonly IUserRepository _repo; + private readonly HttpClient _httpClient; + + public UserService(IUserRepository repo, HttpClient httpClient) + { + _repo = repo; + _httpClient = httpClient; + } + + public User GetUserProfile(int userId) + { + var user = _repo.GetById(userId); + + // Sync-over-async: blocking call + var response = _httpClient.Send(new HttpRequestMessage(HttpMethod.Get, $"/api/avatars/{userId}")); + var avatarUrl = new StreamReader(response.Content.ReadAsStream()).ReadToEnd(); + user.AvatarUrl = avatarUrl; + + return user; + } + + public List GetAllUsers() + { + var users = _repo.GetAll(); + foreach (var user in users) + { + // Blocking call inside a loop + var task = _httpClient.GetStringAsync($"/api/avatars/{user.Id}"); + user.AvatarUrl = task.Result; // Anti-pattern: .Result blocks the thread + } + return users; + } +} + +public class User +{ + public int Id { get; set; } + public string Name { get; set; } = ""; + public string Email { get; set; } = ""; + public string? AvatarUrl { get; set; } +} diff --git a/src/dotnet/tests/refactoring-to-async/eval.yaml b/src/dotnet/tests/refactoring-to-async/eval.yaml new file mode 100644 index 0000000000..b1f87ba9e7 --- /dev/null +++ b/src/dotnet/tests/refactoring-to-async/eval.yaml @@ -0,0 +1,33 @@ +scenarios: + - name: "Refactor synchronous service to async" + prompt: "I have a service class with several synchronous database and HTTP calls. It's causing thread pool starvation under load. Can you convert it to async/await?" + setup: + copy_test_files: true + assertions: + - type: "output_matches" + pattern: "(async|await|Task<)" + - type: "output_matches" + pattern: "(CancellationToken|cancellation)" + - type: "output_not_matches" + pattern: "\\.Result\\b|\\.Wait\\(\\)" + rubric: + - "Identified all synchronous blocking I/O patterns (.Result, .Wait(), synchronous Read/Write calls)" + - "Converted methods bottom-up starting from the lowest-level I/O calls" + - "Changed method signatures to return Task or Task with Async suffix" + - "Added CancellationToken parameter propagation throughout the call chain" + - "Updated interfaces to match the async method signatures" + - "Did not introduce async void methods (except event handlers)" + - "Verified the code compiles after conversion with dotnet build" + expect_tools: ["bash"] + timeout: 120 + + - name: "Async refactoring should not apply to CPU-bound code" + prompt: "I have a method that does heavy matrix multiplication using nested for loops. Can you make it faster?" + assertions: + - type: "output_not_matches" + pattern: "(async Task|await.*ReadAsync|await.*WriteAsync)" + rubric: + - "Did NOT suggest converting CPU-bound computation to async/await" + - "Suggested parallelism approaches (Parallel.For, Task.Run, SIMD, or algorithmic optimization)" + - "Correctly identified that async is for I/O-bound work, not CPU-bound work" + timeout: 60