Skip to content

Async-over-sync and unsafe reflection-based Task.Result access #401

@rido-min

Description

@rido-min

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:82SetAsync wraps a synchronous dictionary operation in Task.Run(() => Set(key, value)). A simple in-memory dictionary set doesn't need a thread pool dispatch. Should use direct execution and return Task.CompletedTask.
  • Teams.Common/Storage/LocalStorage.cs:97DeleteAsync has the same issue with Task.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:23task.GetType().GetProperty("Result")?.GetValue(task) accesses the Result property via reflection. If the task is not completed, this blocks the calling thread. The proper pattern is to await the task first.

Impact

  • Task.Run for trivial sync operations wastes thread pool threads and adds unnecessary scheduling overhead
  • Reflection-based Result access 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;

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions