-
Notifications
You must be signed in to change notification settings - Fork 16
Async-over-sync and unsafe reflection-based Task.Result access #401
Copy link
Copy link
Open
Description
Summary
A few locations wrap synchronous code in Task.Run unnecessarily, and one location accesses Task.Result via reflection which can block unexpectedly.
Locations
Unnecessary Task.Run wrapping sync code
Teams.Common/Storage/LocalStorage.cs:82—SetAsyncwraps a synchronous dictionary operation inTask.Run(() => Set(key, value)). A simple in-memory dictionary set doesn't need a thread pool dispatch. Should use direct execution and returnTask.CompletedTask.Teams.Common/Storage/LocalStorage.cs:97—DeleteAsynchas the same issue withTask.Run(() => Delete(key)).
Note: ExistsAsync and GetAsync in the same class already use the correct pattern (Task.FromResult).
Unsafe reflection-based Result access
Teams.Common/Extensions/MethodInfoExtensions.cs:23—task.GetType().GetProperty("Result")?.GetValue(task)accesses theResultproperty via reflection. If the task is not completed, this blocks the calling thread. The proper pattern is toawaitthe task first.
Impact
Task.Runfor trivial sync operations wastes thread pool threads and adds unnecessary scheduling overhead- Reflection-based
Resultaccess bypasses the compiler's async/await safety checks and can block unexpectedly
Suggested fix
// LocalStorage — replace Task.Run with direct execution
public Task SetAsync(string key, object value)
{
Set(key, value);
return Task.CompletedTask;
}
// MethodInfoExtensions — await the task properly
var result = await (dynamic)task;Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels