Skip to content

Hedging: Fixes rare NullRefreneceException in Headers Clone #5096

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
NaluTripician wants to merge 10 commits into from
Closed

Hedging: Fixes rare NullRefreneceException in Headers Clone #5096

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
53bac39
Added additional safety check + test for clone
NaluTripician Mar 27, 2025
f82096d
Merge branch 'master' into users/nalutripician/hedgingNullRefCloneFix
NaluTripician Mar 31, 2025
2596269
Merge branch 'master' into users/nalutripician/hedgingNullRefCloneFix
NaluTripician Apr 2, 2025
1be0bb7
safer method
NaluTripician Apr 2, 2025
f80735c
Merge branch 'master' into users/nalutripician/hedgingNullRefCloneFix
NaluTripician Apr 4, 2025
cc16cae
Merge branch 'master' into users/nalutripician/hedgingNullRefCloneFix
NaluTripician Apr 4, 2025
d217e71
Merge branch 'master' into users/nalutripician/hedgingNullRefCloneFix
NaluTripician Apr 7, 2025
5551095
Merge branch 'master' into users/nalutripician/hedgingNullRefCloneFix
NaluTripician Apr 8, 2025
5b2e7c7
Merge branch 'master' into users/nalutripician/hedgingNullRefCloneFix
kundadebdatta Apr 15, 2025
59837f3
Merge branch 'master' into users/nalutripician/hedgingNullRefCloneFix
NaluTripician May 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Microsoft.Azure.Cosmos/src/Headers/Headers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,8 @@ internal Headers Clone()
Headers clone = new Headers();
foreach (string key in this.CosmosMessageHeaders.AllKeys())
{
clone.Add(key, this.CosmosMessageHeaders.Get(key));
string value = this.CosmosMessageHeaders.Get(key);
Copy link
Member

@Pilchie Pilchie Mar 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it expected that there be nulls in there? Instead of (or as well as) this should we be investigating how the null got in there?

Copy link
Member

@kirankumarkolli kirankumarkolli Mar 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed that it's a concurrency issue, otherwise +1

Copy link
Member

@Pilchie Pilchie Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What concurrency issue? Someone is writing to the headers collection on one thread while we're reading? That's not safe.

if (value != null) clone.Add(key, this.CosmosMessageHeaders.Get(key));
}

return clone;
Expand Down
81 changes: 81 additions & 0 deletions Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/HeadersTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,5 +171,86 @@ public void TestAllKeysWithKnownProperties()
Assert.IsTrue(allKeys.Contains(HttpConstants.HttpHeaders.Continuation));
Assert.IsTrue(allKeys.Contains(WFConstants.BackendHeaders.SubStatus));
}

[TestMethod]
public void TestHeadersClone()
{
Headers Headers = new Headers();
Headers.CosmosMessageHeaders[Key] = Guid.NewGuid().ToString();
Headers.ContinuationToken = Guid.NewGuid().ToString();
Headers.CosmosMessageHeaders[HttpConstants.HttpHeaders.RetryAfterInMilliseconds] = "20";
Headers.Add(WFConstants.BackendHeaders.SubStatus, "1002");
Headers.PartitionKey = Guid.NewGuid().ToString();
string[] allKeys = Headers.AllKeys();
Assert.IsTrue(allKeys.Contains(Key));
Assert.IsTrue(allKeys.Contains(HttpConstants.HttpHeaders.PartitionKey));
Assert.IsTrue(allKeys.Contains(HttpConstants.HttpHeaders.RetryAfterInMilliseconds));
Assert.IsTrue(allKeys.Contains(HttpConstants.HttpHeaders.Continuation));
Assert.IsTrue(allKeys.Contains(WFConstants.BackendHeaders.SubStatus));

Headers clone = Headers.Clone();
Assert.IsNotNull(clone);
string[] keys = clone.AllKeys();
Assert.IsTrue(keys.Contains(Key));
Assert.IsTrue(keys.Contains(HttpConstants.HttpHeaders.PartitionKey));
Assert.IsTrue(keys.Contains(HttpConstants.HttpHeaders.RetryAfterInMilliseconds));
Assert.IsTrue(keys.Contains(HttpConstants.HttpHeaders.Continuation));
Assert.IsTrue(keys.Contains(WFConstants.BackendHeaders.SubStatus));
Assert.AreEqual(allKeys.Length, keys.Length);
Assert.AreEqual(
Headers.Get(HttpConstants.HttpHeaders.PartitionKey),
clone.Get(HttpConstants.HttpHeaders.PartitionKey));
Assert.AreEqual(
Headers.Get(HttpConstants.HttpHeaders.RetryAfterInMilliseconds),
clone.Get(HttpConstants.HttpHeaders.RetryAfterInMilliseconds));
Assert.AreEqual(
Headers.Get(HttpConstants.HttpHeaders.Continuation),
clone.Get(HttpConstants.HttpHeaders.Continuation));
Assert.AreEqual(
Headers.Get(WFConstants.BackendHeaders.SubStatus),
clone.Get(WFConstants.BackendHeaders.SubStatus));
}

[TestMethod]
public void TestNullHeadersClone()
{
Headers Headers = new Headers();
Headers.CosmosMessageHeaders[Key] = Guid.NewGuid().ToString();
Headers.ContinuationToken = Guid.NewGuid().ToString();
Headers.CosmosMessageHeaders[HttpConstants.HttpHeaders.RetryAfterInMilliseconds] = "20";
Headers.Add(WFConstants.BackendHeaders.SubStatus, "1002");
Headers.PartitionKey = "pk";

Headers.CosmosMessageHeaders["x-ms-documentdb-partitionkey"] = null;

string[] allKeys = Headers.AllKeys();
Assert.IsTrue(allKeys.Contains(Key));
Assert.IsFalse(allKeys.Contains(HttpConstants.HttpHeaders.PartitionKey));
Assert.IsTrue(allKeys.Contains(HttpConstants.HttpHeaders.RetryAfterInMilliseconds));
Assert.IsTrue(allKeys.Contains(HttpConstants.HttpHeaders.Continuation));
Assert.IsTrue(allKeys.Contains(WFConstants.BackendHeaders.SubStatus));

Headers clone = Headers.Clone();
Assert.IsNotNull(clone);
string[] keys = clone.AllKeys();
Assert.IsTrue(keys.Contains(Key));
Assert.IsFalse(keys.Contains(HttpConstants.HttpHeaders.PartitionKey));
Assert.IsTrue(keys.Contains(HttpConstants.HttpHeaders.RetryAfterInMilliseconds));
Assert.IsTrue(keys.Contains(HttpConstants.HttpHeaders.Continuation));
Assert.IsTrue(keys.Contains(WFConstants.BackendHeaders.SubStatus));
Assert.AreEqual(allKeys.Length, keys.Length);
Assert.AreEqual(
Headers.Get(HttpConstants.HttpHeaders.PartitionKey),
clone.Get(HttpConstants.HttpHeaders.PartitionKey));
Assert.AreEqual(
Headers.Get(HttpConstants.HttpHeaders.RetryAfterInMilliseconds),
clone.Get(HttpConstants.HttpHeaders.RetryAfterInMilliseconds));
Assert.AreEqual(
Headers.Get(HttpConstants.HttpHeaders.Continuation),
clone.Get(HttpConstants.HttpHeaders.Continuation));
Assert.AreEqual(
Headers.Get(WFConstants.BackendHeaders.SubStatus),
clone.Get(WFConstants.BackendHeaders.SubStatus));
}
}
}
Loading