Skip to content
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.

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

Fix NullReferenceExceptions during serialization #1081

Merged
merged 7 commits into from
Dec 10, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
2 changes: 1 addition & 1 deletion Microsoft.Azure.Cosmos/src/CosmosClientOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ internal Protocol ConnectionProtocol
/// </summary>
internal CosmosSerializer GetCosmosSerializerWithWrapperOrDefault()
{
if (this.SerializerOptions != null)
if (this.SerializerOptions != null && !this.SerializerOptions.IsDefaultSettings())
{
CosmosJsonDotNetSerializer cosmosJsonDotNetSerializer = new CosmosJsonDotNetSerializer(this.SerializerOptions);
return new CosmosJsonSerializerWrapper(cosmosJsonDotNetSerializer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

namespace Microsoft.Azure.Cosmos
{
using System;
using System.Diagnostics;
using System.IO;
using System.Text;
using Newtonsoft.Json;
Expand All @@ -15,7 +17,7 @@ namespace Microsoft.Azure.Cosmos
internal sealed class CosmosJsonDotNetSerializer : CosmosSerializer
{
private static readonly Encoding DefaultEncoding = new UTF8Encoding(false, true);
private readonly JsonSerializer Serializer;
private readonly JsonSerializerSettings SerializerSettings;

/// <summary>
/// Create a serializer that uses the JSON.net serializer
Expand All @@ -26,7 +28,7 @@ internal sealed class CosmosJsonDotNetSerializer : CosmosSerializer
/// </remarks>
internal CosmosJsonDotNetSerializer()
{
this.Serializer = JsonSerializer.Create();
this.SerializerSettings = null;
}

/// <summary>
Expand All @@ -38,6 +40,11 @@ internal CosmosJsonDotNetSerializer()
/// </remarks>
internal CosmosJsonDotNetSerializer(CosmosSerializationOptions cosmosSerializerOptions)
{
if (cosmosSerializerOptions == null || cosmosSerializerOptions.IsDefaultSettings())
{
throw new ArgumentException("No need to create a custom serializer. The settings are the default.");
}
j82w marked this conversation as resolved.
Show resolved Hide resolved

JsonSerializerSettings jsonSerializerSettings = new JsonSerializerSettings()
{
NullValueHandling = cosmosSerializerOptions.IgnoreNullValues ? NullValueHandling.Ignore : NullValueHandling.Include,
Expand All @@ -47,7 +54,7 @@ internal CosmosJsonDotNetSerializer(CosmosSerializationOptions cosmosSerializerO
: null
};

this.Serializer = JsonSerializer.Create(jsonSerializerSettings);
this.SerializerSettings = jsonSerializerSettings;
}

/// <summary>
Expand All @@ -59,7 +66,7 @@ internal CosmosJsonDotNetSerializer(CosmosSerializationOptions cosmosSerializerO
/// </remarks>
internal CosmosJsonDotNetSerializer(JsonSerializerSettings jsonSerializerSettings)
{
this.Serializer = JsonSerializer.Create(jsonSerializerSettings);
this.SerializerSettings = jsonSerializerSettings ?? throw new ArgumentNullException(nameof(jsonSerializerSettings));
}

/// <summary>
Expand All @@ -81,7 +88,8 @@ public override T FromStream<T>(Stream stream)
{
using (JsonTextReader jsonTextReader = new JsonTextReader(sr))
{
return this.Serializer.Deserialize<T>(jsonTextReader);
JsonSerializer jsonSerializer = this.GetSerializer();
return jsonSerializer.Deserialize<T>(jsonTextReader);
}
}
}
Expand All @@ -101,7 +109,8 @@ public override Stream ToStream<T>(T input)
using (JsonWriter writer = new JsonTextWriter(streamWriter))
{
writer.Formatting = Newtonsoft.Json.Formatting.None;
this.Serializer.Serialize(writer, input);
JsonSerializer jsonSerializer = this.GetSerializer();
jsonSerializer.Serialize(writer, input);
writer.Flush();
streamWriter.Flush();
}
Expand All @@ -110,5 +119,14 @@ public override Stream ToStream<T>(T input)
streamPayload.Position = 0;
return streamPayload;
}

/// <summary>
/// JsonSerializer has hit a race conditions with custom settings that cause null reference exception.
/// To avoid the race condition a new JsonSerializer is created for each call
/// </summary>
private JsonSerializer GetSerializer()
{
return JsonSerializer.Create(this.SerializerSettings);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,19 @@ namespace Microsoft.Azure.Cosmos
/// </summary>
public sealed class CosmosSerializationOptions
{
kirankumarkolli marked this conversation as resolved.
Show resolved Hide resolved
private const bool DefaultIgnoreNullValues = false;
private const bool DefaultIndented = false;
private const CosmosPropertyNamingPolicy DefaultCosmosPropertyNamingPolicy = CosmosPropertyNamingPolicy.Default;

/// <summary>
/// Create an instance of CosmosSerializationOptions
/// with the default values for the Cosmos SDK
/// </summary>
public CosmosSerializationOptions()
{
this.IgnoreNullValues = false;
this.Indented = false;
this.PropertyNamingPolicy = CosmosPropertyNamingPolicy.Default;
this.IgnoreNullValues = DefaultIgnoreNullValues;
this.Indented = DefaultIndented;
this.PropertyNamingPolicy = DefaultCosmosPropertyNamingPolicy;
}

/// <summary>
Expand All @@ -47,5 +51,15 @@ public CosmosSerializationOptions()
/// The default value is CosmosPropertyNamingPolicy.Default
/// </remarks>
public CosmosPropertyNamingPolicy PropertyNamingPolicy { get; set; }

/// <summary>
/// Helper method to check if all the setting values are the default.
/// </summary>
internal bool IsDefaultSettings()
{
return this.IgnoreNullValues == DefaultIgnoreNullValues &&
this.Indented == DefaultIndented &&
this.PropertyNamingPolicy == DefaultCosmosPropertyNamingPolicy;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,27 @@ namespace Microsoft.Azure.Cosmos.Performance.Tests.Benchmarks
{
using System;
using System.IO;
using System.Text;
using System.Threading.Tasks;
using BenchmarkDotNet.Attributes;
using Microsoft.Azure.Cosmos;
using Microsoft.Azure.Documents.Collections;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;

/// <summary>
/// Benchmark for Item related operations.
/// </summary>
[MemoryDiagnoser]
public class ItemBenchmark
{
private static readonly Encoding DefaultEncoding = new UTF8Encoding(false, true);
private readonly CosmosClient clientForTests;
private readonly Container container;
private readonly JsonSerializer jsonSerializer = new JsonSerializer();
private JObject baseItem;
private byte[] payloadBytes;

private dynamic TestItem;
/// <summary>
/// Initializes a new instance of the <see cref="ItemBenchmark"/> class.
/// </summary>
Expand All @@ -37,6 +43,53 @@ public ItemBenchmark()
this.payloadBytes = ms.ToArray();
}
}

this.TestItem = new
{
id = "test",
pk = "what",
value = 1245,
stop = true,
};
}

[Benchmark]
public void StaticSerializeItem()
kirankumarkolli marked this conversation as resolved.
Show resolved Hide resolved
{
MemoryStream streamPayload = new MemoryStream();
using (StreamWriter streamWriter = new StreamWriter(streamPayload, encoding: ItemBenchmark.DefaultEncoding, bufferSize: 1024, leaveOpen: true))
{
using (JsonWriter writer = new JsonTextWriter(streamWriter))
{
writer.Formatting = Newtonsoft.Json.Formatting.None;
this.jsonSerializer.Serialize(writer, this.TestItem);
writer.Flush();
streamWriter.Flush();
}
}

streamPayload.Position = 0;
streamPayload.Dispose();
}

[Benchmark]
public void NewSerializeItem()
kirankumarkolli marked this conversation as resolved.
Show resolved Hide resolved
{
MemoryStream streamPayload = new MemoryStream();
using (StreamWriter streamWriter = new StreamWriter(streamPayload, encoding: ItemBenchmark.DefaultEncoding, bufferSize: 1024, leaveOpen: true))
{
using (JsonWriter writer = new JsonTextWriter(streamWriter))
{
writer.Formatting = Newtonsoft.Json.Formatting.None;
JsonSerializer jsonSerializer = new JsonSerializer();
jsonSerializer.Serialize(writer, this.TestItem);
writer.Flush();
streamWriter.Flush();
}
}

streamPayload.Position = 0;
streamPayload.Dispose();
}

/// <summary>
Expand Down Expand Up @@ -83,7 +136,7 @@ public async Task UpsertItem()
[Benchmark]
public async Task UpsertItemStream()
{

ResponseMessage response = await this.container.UpsertItemStreamAsync(
new MemoryStream(this.payloadBytes),
new Cosmos.PartitionKey(Constants.ValidOperationId));
Expand Down Expand Up @@ -134,7 +187,7 @@ public async Task UpdateItem()
{
ResponseMessage response = await this.container.ReplaceItemStreamAsync(
new MemoryStream(this.payloadBytes),
Constants.ValidOperationId,
Constants.ValidOperationId,
new Cosmos.PartitionKey(Constants.ValidOperationId));
if (response.StatusCode == System.Net.HttpStatusCode.NotFound || response.Content == null)
{
Expand Down