SendGridClient thread-safety documentation

[william-gross]

Please state in the documentation whether it is safe to create a single SendGridClient object and share it among all threads in an app domain. In particular, I'd like to know if it is safe for multiple threads in an ASP.NET application to call SendEmailAsync or RequestAsync at the same time.

I believe this was safe in version 6.3.4 (with SendGrid.Web.DeliverAsync) and it's probably still safe now (since the underlying implementation is HttpClient), but it would be nice to know for sure.

[thinkingserious]

Hello @william-gross,

While I have not done any specific testing for thread safety, I believe your assumption is correct with regards to [HttpClient](https://msdn.microsoft.com/en-us/library/system.net.http.httpclient(v=vs.110).aspx#Anchor_5).

I will leave this thread open so that we can create a test that verifies and documents thread safety with a real world example.

For this issue to gain priority in our backlog, we need additional +1's or a PR. When we receive a PR, that provides the biggest jump in priority.

With Best Regards,

Elmer

[paultechguy]

I'd like to know this as well for DI purposes.

[thinkingserious]

Thanks @paultechguy,

I've add your vote to our backlog.

[winzig]

I would also like to have this confirmed.

[thinkingserious]

Thanks for the upvote @winzig.

[alexmaie]

Another Up from me 👍 :)

[profet23]

Still no confirmation?

[thinkingserious]

Hi @profet23,

Not yet, but I've added your vote to help this issue rise in priority. Thanks!

With Best Regards,

Elmer

[kierenj]

Please 👍

[thinkingserious]

Thanks for the vote @kierenj!

[twomm]

Up

[MSC-AA]

@thinkingserious Could you please update us on the issue?

[PureKrome]

Also a vote from me, etc! Can we Singleton or Scope this? etc..

[thinkingserious]

Hello @MSC-AA,

This one is still on our backlog. We are hoping to increase our resources internally soon to help us power through our backlog faster. If you know anyone, please let us know at dx@sendgrid.com.

Thanks for checking in!

With Best Regards,

Elmer

[thinkingserious]

Referencing: #847

[rodion-m]

I want to find out it too, can we use Singleton or not.

[kntajus]

Another vote from me!

[biliktamas79]

Another vote from me!

[CleytonGoncalves]

Me too!

[HoomanBahreini]

It's very strange that SendGrid does not know if their own client is thread safe or not...

This is important details, e.g. Microsoft's DbContext is not thread safe, so we need to instantiate a new instance every time we need it, on the other hand Elasticsearch client is thread safe, so we need to reuse the same instance for the entire lifetime of the application. Getting this wrong would have severe consequences, for example Elasticsearch does all the caching in the client, so if we keep instantiating a new instance we won't have any cache.

Unfortunately, people who come across this thread would lose confidence in your product...

[NoahStahl]

Given that the SendGridClient class encapsulates a HttpClient instance field, it would seem to me that using it as a singleton would at least potentially have the issues documented with HttpClient singletons:

Another issue that developers run into is when using a shared instance of HttpClient in long running processes. In a situation where the HttpClient is instantiated as a singleton or a static object, it fails to handle the DNS changes as described in this issue of the dotnet/corefx GitHub repository.

So at least from that perspective, it seems wise to use scoped lifetime even if otherwise thread safe?

[ssougnez]

Up and also up for the comment of @HoomanBahreini... The question has been asked 3 years ago and no one is able to say whether SendGrid is thread safe or not o_O That's insane...

[PureKrome]

@ssougnez I was going to come in here, agree with you AND also suggest that the lack of any response speaks volumes about SG and what they think about .NET Devs (with respect to using/consuming their product).

--BUT-- 4 days go @childish-sambino changes the tags on this issue ... which actually looks like the first type of interaction (on this thread) since Q3 2018 - nearly 2 years ago.

so maybe.... they are interested again? not sure.

It's pretty frustrating, none-the-less.

[shoter]

SendEmailAsync is thread safe.

First thing it does is

 return await this.RequestAsync(
                Method.POST,
                msg.Serialize(),
                urlPath: "mail/send",
                cancellationToken: cancellationToken).ConfigureAwait(false);

Nothing unsafe for threads here. Using only local variables.

Then it is executing method from base class. I assume that fields/props inside this.options does not change.
The method contains following operations:

var baseAddress = new Uri(this.options.Host);
            if (!baseAddress.OriginalString.EndsWith("/"))
            {
                baseAddress = new Uri(baseAddress.OriginalString + "/");
            }

Creates local var from readonly object - thread safe

// Build the final request
            var request = new HttpRequestMessage
            {
                Method = new HttpMethod(method.ToString()),
                RequestUri = new Uri(baseAddress, this.BuildUrl(urlPath, queryParams)),
                Content = requestBody == null ? null : new StringContent(requestBody, Encoding.UTF8, this.MediaType),
            };

creates local variable from method params - thread safe.

// Drop the default UTF-8 content type charset for JSON payloads since some APIs may not accept it.
            if (request.Content != null && this.MediaType == DefaultMediaType)
            {
                request.Content.Headers.ContentType.CharSet = null;
            }

            // set header overrides
            if (this.options.RequestHeaders?.Count > 0)
            {
                foreach (var header in this.options.RequestHeaders)
                {
                    request.Headers.TryAddWithoutValidation(header.Key, header.Value);
                }
            }

Does some operations on local object with help of readonly field - thread safe.

  // set standard headers
            request.Headers.Authorization = this.options.Auth;
            request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue(this.MediaType));
            request.Headers.UserAgent.TryParseAdd($"sendgrid/{ClientVersion} csharp");
            return await this.MakeRequest(request, cancellationToken).ConfigureAwait(false);

Thread safe - we are only operating on local object.
Let's dive into this.MakeRequest

 HttpResponseMessage response = await this.client.SendAsync(request, cancellationToken).ConfigureAwait(false);

This is thread safe op.

if (!response.IsSuccessStatusCode && this.options.HttpErrorAsException)
            {
                await ErrorHandler.ThrowException(response).ConfigureAwait(false);
            }

using local variable and readonly thingy - thread safe.

/ Correct invalid UTF-8 charset values returned by API
            if (response.Content?.Headers?.ContentType?.CharSet == "utf8")
            {
                response.Content.Headers.ContentType.CharSet = Encoding.UTF8.WebName;
            }

Only local variable - thread safe.

/ Correct invalid UTF-8 charset values returned by API
            if (response.Content?.Headers?.ContentType?.CharSet == "utf8")
            {
                response.Content.Headers.ContentType.CharSet = Encoding.UTF8.WebName;
            }

Only local var - thread safe.

---

I did not find anything thread unsafe even if you share httpClient (but i suggest using httpclientfactory).