Summary
The in-memory test data store InMemoryTaskStore used by McpServerTaskTests is not thread-safe, resulting in occasional test failures due to field mutation races between test background tasks and server polling.
Symptoms:
- Flaky failures in
CallToolAsync_AsyncTool_FailedTask_ThrowsMcpException (and similar tests)
- Stack trace shows
Nullable object must have a value thrown from GetTask when Status == Failed but Error is null.
- The client receives a
McpProtocolException (internal error) instead of the expected McpException for failed tasks.
How to Reproduce:
- Run the test suite repeatedly (e.g., on CI, especially macOS)
- Failing run shows error:
Assert.Throws() Failure: Exception type was not an exact match
Expected: typeof(ModelContextProtocol.McpException)
Actual: typeof(ModelContextProtocol.McpProtocolException)
System.InvalidOperationException: Nullable object must have a value.
at InMemoryTaskStore.GetTask(...)
Root Cause
- The test's
FailTask sets entry.Status = Failed before entry.Error, so a concurrent reader (GetTask, called by the server handler) can see a failed status with Error == null.
GetTask then dereferences entry.Error!.Value, which throws.
- Similar races exist in other methods mutating/testing
TaskEntry fields.
Solution
- Make
InMemoryTaskStore atomic for each operation: lock around mutations and reads, or replace ConcurrentDictionary + mutable TaskEntry fields with an immutable snapshot approach.
- For a minimal fix: add a private
object _gate lock to synchronize all mutations and reads, and always lock the critical section.
- Also reorder assignments in
FailTask etc. so all fields are valid before status is set.
Code suggestion
private readonly object _gate = new();
// ...
public GetTaskResult GetTask(string taskId)
{
lock (_gate)
{
// ... see full solution in assistant's comment ...
}
}
// ... lock everywhere else ...
Impact
- This causes rare but real test failures on CI, especially with concurrent test scheduling.
References
- Failing job: Build and Test matrix in
.github/workflows/ci-build-test.yml
- Example failure:
CallToolAsync_AsyncTool_FailedTask_ThrowsMcpException in McpServerTaskTests.cs
Summary
The in-memory test data store
InMemoryTaskStoreused byMcpServerTaskTestsis not thread-safe, resulting in occasional test failures due to field mutation races between test background tasks and server polling.Symptoms:
CallToolAsync_AsyncTool_FailedTask_ThrowsMcpException(and similar tests)Nullable object must have a valuethrown fromGetTaskwhenStatus == FailedbutError is null.McpProtocolException(internal error) instead of the expectedMcpExceptionfor failed tasks.How to Reproduce:
Root Cause
FailTasksetsentry.Status = Failedbeforeentry.Error, so a concurrent reader (GetTask, called by the server handler) can see a failed status withError == null.GetTaskthen dereferencesentry.Error!.Value, which throws.TaskEntryfields.Solution
InMemoryTaskStoreatomic for each operation: lock around mutations and reads, or replaceConcurrentDictionary+ mutableTaskEntryfields with an immutable snapshot approach.object _gatelock to synchronize all mutations and reads, and always lock the critical section.FailTasketc. so all fields are valid before status is set.Code suggestion
Impact
References
.github/workflows/ci-build-test.ymlCallToolAsync_AsyncTool_FailedTask_ThrowsMcpExceptioninMcpServerTaskTests.cs