Potential non thread-safety in DefaultPlatformInformation.ReadEnviromentVariable function

KuSh commented 1 year ago

Description

When launching multiple orchestration function at a time we often get following exception :

Message: Exception while executing function: Functions.analyzer Exception binding parameter 'context' An item with the same key has already been added. Key: ExternalDurablePowerShellSDK Exception type: System.ArgumentException Failed method: System.ThrowHelper.ThrowAddingDuplicateWithKeyArgumentException

After digging in your source code it seems DefaultPlatformInformation.ReadEnviromentVariable function is not thread safe and can generate such exception if a thread add the variable to cachedEnviromentVariables after another thread has called TryGetValue but before it calls Add

Expected behavior

Launching multiple orchestration in a short time frame should not generate failures

Actual behavior

Orchestrator has Failed RuntimeStatus with Orchestrator function 'analyzer' failed: One or more errors occurred. (Exception while executing function: Functions.analyzer) Output

Relevant source code snippets

https://github.com/Azure/azure-functions-durable-extension/blob/c82b72271630741664104cefdc8c69e06b4d4aed/src/WebJobs.Extensions.DurableTask/DefaultPlatformInformation.cs#L34-L44

App Details

Durable Functions extension version (e.g. v1.8.3): 2.9.3
Azure Functions runtime version (1.0 or 2.0): 2.0
Programming language used: typescript

Screenshots

If applicable, add screenshots to help explain your problem.

If deployed to Azure

We have access to a lot of telemetry that can help with investigations. Please provide as much of the following information as you can to help us investigate!

Timeframe issue observed: 2023-06-23T07:16:37.5796559Z
Function App name:
Function name(s): analyzer
Azure region: west-europe
Orchestration instance ID(s): 64905c37fbdb7ceb18f5df1c
Azure storage account name:

If you don't want to share your Function App or storage account name GitHub, please at least share the orchestration instance ID. Otherwise it's extremely difficult to look up information.

dlecan commented 10 months ago

Any news on this bug? Thanks

bichuga commented 10 months ago

Looks like a super easy fix...

private ConcurrentDictionary<string, string?> cachedEnvironmentVariables = new ConcurrentDictionary<string, string?>();

private string? ReadEnvironmentVariable(string variableName)
{
    return cachedEnvironmentVariables.GetOrAdd(variableName, key => this.nameResolver.Resolve(key));
}

Azure / azure-functions-durable-extension