Skip to content

Commit 4cbe83b

Browse files
NaluTripicianCopilotkirankumarkolli
authored
[FaultInjection] FaultInjection: Fixes critical bugs for release 2 (#5675)
## Summary This PR addresses critical bug fixes in the FaultInjection library identified during the pre-release analysis for Release 2. ### Changes **1. Thread-safety fix in `FaultInjectionApplicationContext.AddRuleExecution()` (#5653)** - The `AddRuleExecution()` method was using `TryAdd` + direct `List<T>.Add()` on values stored in `ConcurrentDictionary`, which is not thread-safe. Replaced with `AddOrUpdate()` with `lock` synchronization on the list instances. **2. SendDelay validation in `FaultInjectionServerErrorResultBuilder.Build()` (#5654)** - `Build()` validated that a delay was set for `ResponseDelay` and `ConnectionDelay` error types, but not for `SendDelay`. Users could create a `SendDelay` rule without specifying a delay (defaulting to `TimeSpan.Zero`). Added `SendDelay` to the validation check. **3. Incorrect `OutputType` in `FaultInjection.csproj` (#5655)** - The csproj had `<OutputType>Exe</OutputType>` and `<TargetType>library</TargetType>` which is contradictory. Removed both lines since the default for SDK-style projects is `Library`. **4. Null validation in `FaultInjector` constructor (#5656)** - The constructor accepted `null` for the `rules` parameter without validation, leading to a deferred `NullReferenceException`. Added an `ArgumentNullException` guard. ### Testing - Build verified locally with `dotnet build` - All changes are minimal and surgical Parent tracking issue: #5652 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Kiran Kumar Kolli <kirankk@microsoft.com>
1 parent d8c473f commit 4cbe83b

4 files changed

Lines changed: 29 additions & 11 deletions

File tree

Microsoft.Azure.Cosmos/FaultInjection/src/FaultInjection.csproj

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@
77
<Copyright>© Microsoft Corporation. All rights reserved.</Copyright>
88
<NeutralLanguage>en-US</NeutralLanguage>
99
<CurrentDate>$([System.DateTime]::Now.ToString(yyyyMMdd))</CurrentDate>
10-
<OutputType>Exe</OutputType>
1110
<TargetFramework>net6</TargetFramework>
12-
<TargetType>library</TargetType>
1311
<Authors>Microsoft</Authors>
1412
<RootNamespace>Microsoft.Azure.Cosmos.FaultInjection</RootNamespace>
1513
<AssemblyName>Microsoft.Azure.Cosmos.FaultInjection</AssemblyName>

Microsoft.Azure.Cosmos/FaultInjection/src/FaultInjectionServerErrorResultBuilder.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ public FaultInjectionServerErrorResultBuilder WithInjectionRate(double injection
9797
public FaultInjectionServerErrorResult Build()
9898
{
9999
if ((this.serverErrorType == FaultInjectionServerErrorType.ResponseDelay
100-
|| this.serverErrorType == FaultInjectionServerErrorType.ConnectionDelay)
100+
|| this.serverErrorType == FaultInjectionServerErrorType.ConnectionDelay
101+
|| this.serverErrorType == FaultInjectionServerErrorType.SendDelay)
101102
&& !this.isDelaySet)
102103
{
103104
throw new ArgumentNullException(nameof(this.delay), "Argument 'delay' required for server error type: " + this.serverErrorType);

Microsoft.Azure.Cosmos/FaultInjection/src/FaultInjector.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ public class FaultInjector : IFaultInjector
1616

1717
public FaultInjector(List<FaultInjectionRule> rules)
1818
{
19+
if (rules == null)
20+
{
21+
throw new ArgumentNullException(nameof(rules));
22+
}
23+
1924
this.chaosInterceptorFactory = new ChaosInterceptorFactory(rules);
2025
}
2126

Microsoft.Azure.Cosmos/FaultInjection/src/implementation/FaultInjectionApplicationContext.cs

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,29 @@ public FaultInjectionApplicationContext()
2626

2727
internal void AddRuleExecution(string ruleId, Guid activityId)
2828
{
29-
if(!this.executionsByRuleId.TryAdd(ruleId, new List<(DateTime, Guid)>() { (DateTime.UtcNow, activityId) }))
30-
{
31-
this.executionsByRuleId[ruleId].Add((DateTime.UtcNow, activityId));
32-
}
29+
this.executionsByRuleId.AddOrUpdate(
30+
ruleId,
31+
_ => new List<(DateTime, Guid)>() { (DateTime.UtcNow, activityId) },
32+
(_, existing) =>
33+
{
34+
lock (existing)
35+
{
36+
existing.Add((DateTime.UtcNow, activityId));
37+
}
38+
return existing;
39+
});
3340

34-
if (!this.executionsByActivityId.TryAdd(activityId, new List<(DateTime, string)>() { (DateTime.UtcNow, ruleId) }))
35-
{
36-
this.executionsByActivityId[activityId].Add((DateTime.UtcNow, ruleId));
37-
}
41+
this.executionsByActivityId.AddOrUpdate(
42+
activityId,
43+
_ => new List<(DateTime, string)>() { (DateTime.UtcNow, ruleId) },
44+
(_, existing) =>
45+
{
46+
lock (existing)
47+
{
48+
existing.Add((DateTime.UtcNow, ruleId));
49+
}
50+
return existing;
51+
});
3852

3953
this.values.Add((DateTime.UtcNow, ruleId, activityId));
4054
}

0 commit comments

Comments
 (0)