Problem on latest SDKs

The following function SetCouchbaseEntry runs just fine on 3.1.3 version

public async Task<IMutationResult> Couchbase_Uppsert<T>(ICouchbaseCollection _collection, string key, T entry, DateTime utcExpiry, long transcoder) {
            try {
                var result = await _collection.UpsertAsync(key, entry,
                    options => {
                        options.Expiry(utcExpiry - DateTime.Now);
                        options.Timeout(TimeSpan.FromSeconds(5));
                        if (transcoder == 0) {
                            options.Transcoder(new LegacyTranscoder());
                        }
                        else if (transcoder == 1) {
                            options.Transcoder(new RawStringTranscoder());
                        }
                        else if (transcoder == 2) {
                            options.Transcoder(new RawBinaryTranscoder());
                        }
                        else if (transcoder == 3) {
                            options.Transcoder(new JsonTranscoder());
                        }
                    }
                ).ConfigureAwait(false);

                return (result);
            }
            catch (Exception) {
            }

            return (null);
        }

        public IMutationResult SetCouchbaseEntry<T>(ICouchbaseCollection _collection, String CacheStr, T strValue, long iMinutes, long transcoder = 0) {
            IMutationResult result = null;
            try {
                result = Couchbase_Uppsert<T>(_collection, CacheStr, strValue, DateTime.Now.AddMinutes(iMinutes), transcoder).GetAwaiter().GetResult();
            }
            catch (Exception E) {
                ;
            }

            return (result);
        }

After 3.1.3 it does not run. Actually it just hungs on await _collection.UpsertAsync and never returns.

What has changed? Can I do anything to fix it?

Is the .GetAwaiter().GetResult() the problem? You mentioned that on your own docs at
https://docs.couchbase.com/dotnet-sdk/current/howtos/concurrent-async-apis.html
on section Synchronous Programming using Task.Result

Hello @zissop1 which server version are you using and did you try the latest version of .NET SDK (3.2.4) ?

@zissop1

I think you’re correct, the most likely problem is using GetAwaiter().GetResult(). This approach, also known as the “sync over async antipattern”, should be avoided if at all possible. It is specifically known to cause issues like this where things hang.

Sometimes there’s no way around it due to interop with code that can’t be made asynchronous. In that case, there are some patterns you can find on the internet that assist, such as AsyncHelper.RunSync, but these come with their own caveats. They can cause thread pool depletion, among other issues.

I’d recommend making SetCouchbaseEntry<T> asynchronous, if you can.

I have tested it on all newer versions, same behavior.
As I said the GetAwaiter().GetResult() is already a valid pattern and is mentioned even on your documents. So you are aware that is a valid pattern. (Choosing an API | Couchbase Docs)
Please try to check what has changed after 3.1.3 that invalidates this well know pattern. Something has to have changed and maybe the fix is not something difficult. What if someone needs to make synchronous like calls? Do they have to use libraries as Mixed Reality Toolkit Unity and are you sure this is going to work and its not the same thing?
For me and i assume many many other all the way async is not an option.
Avoid that is not the same as “not supported”. Here it seems it is not supported at all due to internal code changes of the SDK. If it is possible please find what has changed and why this has broken.
Finally is there a Microsoft doc indicates that this can cause hangs? I have not found anything about this, it is mentioned as a possible problem only if .ConfigureAwait is called with parameter true (same context). Not other case.

That code runs just fine

private async Task<ICouchbaseCollection> InitializeClusterAsync(String Uri, String Bucket, String Username, String Password) {
            var options = new ClusterOptions() {
                KvTimeout = TimeSpan.FromSeconds(5),
                ManagementTimeout = TimeSpan.FromSeconds(12),
                EnableDnsSrvResolution = false,
                UserName = Username,
                Password = Password,
                EnableTls = false,
                EnableConfigPolling = false,
                MaxKvConnections = 25
            };

            options = new ClusterOptions() {
                UserName = Username,
                Password = Password
            };

            var cluster = await Couchbase.Cluster.ConnectAsync(Uri, options).ConfigureAwait(false);
            var bucket = await cluster.BucketAsync(Bucket).ConfigureAwait(false);

            return (bucket.DefaultCollection());
        }

        public ICouchbaseCollection GetCouchbaseCollection(String Uri, String Bucket, String Username, String Password) {
            _gcollection = InitializeClusterAsync(Uri, Bucket, Username, Password).GetAwaiter().GetResult();
            return (_gcollection);
        }

I realize it’s in the documentation, but in my opinion it should not be. I believe it was added to help people converting from SDK 2.x applications who are used to having synchronous methods. But it really is an antipattern except in some very precise circumstances.

The main issue is that it blocks the current thread. If you are running within a SynchronizationContext at the time you call the method, then any uses of await within the async method being called will, by default, attempt to run continuations on the SynchronizationContext. Many implementations of SynchronizationContext (especially in legacy ASP.NET, WPF, WinForms, etc) only allow a single task to execute at a time. The calling thread is blocking the SynchronizationContext, so then the task continuations are blocked waiting on it to become available, which it never will, and it hangs. Even if there isn’t a limiting SynchronizationContext in place, it may still hang under load due to thread pool depletion.

Here’s an article that covers a lot of the various issues: Understanding Async, Avoiding Deadlocks in C# | by Eke Péter | Rubrikk Group | Medium

Note that hangs can be caused by Task.Wait(), Task.Result, or Task.GetAwaiter().GetResult(), they all have the same issue where they block the calling thread waiting for completion. Task.GetAwaiter().GetResult() is basically the same as Task.Result except that it applies await style semantics for rethrowing exceptions if the task fails.

1 Like

Without synchronization context is should not be an issue. ConfigureAwait(false) defines that no synchronization context is being used,
Thread pool depletion is not an issue here either, i used it on scenarios with a simple call without other programs to use couchbase.
Fianlly what changed from 3.13 that can cause this issue?

ConfigureAwait(false) is not the end-all fix-everything for SynchronizationContext, unfortunately. It affects that specific await only, saying that if it does need to complete asynchronously to not use the SynchronizationContext. To be completely effective, however, you must include it on EVERY await down the call stack. It doesn’t guarantee that the SynchronizationContext is dropped for the continuation because the continuation may be synchronous instead.

In order to try to reduce this problem, the Couchbase SDK does try to use .ConfigureAwait(false) throughout. However, it is very easy to miss. Some code change that missed ConfigureAwait(false) is, in fact, the likely culprit for the change in behavior after 3.1.3.

My point is that relying on that behavior isn’t safe. It could get fixed in the next release, and another spot could appear in the next release after that. Also, race conditions can occur that might make a missed spot not apparent until your application is under load.

If you really must use .GetAwaiter().GetResult() and rewriting to true async isn’t an option, then I strongly recommend using a pattern like AsyncHelper.RunSync which takes some performance penalties in exchange for the guarantee that it won’t deadlock.

I will try to use AsyncHelper.RunSync and report back.
If you find any place you miss ConfigureAwait(false) please fix it, note that other data operation are being hang, for example GetAsync…
All how diffucult is to support a set of synchronous operations for key value operations? This would be nice.

I think you are right on the possible bug on the SDK.
I downloaded the recent version from Github. Tried to locate async functions called with await keyword with no ConfigureAwait(false) attached to them.
Fix as many as i found and the new assembly seems to work now.
So please do the same on the next release, find all await without ConfigureAwait(false) and attach it to them, seems fixes the problems.

As i see i fix that on
Couchbase/CouchbaseBucket.cs
Couchbase/Core/Retry/RetryOrchestrator.cs
Couchbase/KeyValue/CouchbaseCollection.cs

but maybe the problem is on other places too.

I filed an issue to track this: [NCBC-3009] Add missing ConfigureAwait(false) calls to reduce deadlocks - Couchbase database

And I’ve pushed up a changeset for review: https://review.couchbase.org/c/couchbase-net-client/+/165689

The good news is that I found a code analyzer I could turn on that should hopefully prevent future regressions. That said, I still feel you should look into lower-risk patterns for calling async methods from sync code. This problem is far from the only disadvantage from using your current pattern. But, regardless, these changes should help.

1 Like