Async / MARS issues editing part definitions with queries on SQL Server

[deanmarcussen]

Only occurs on SQL Server

Repro

• Create a ListPart
• Edit settings to EnableOrder

4 times out of 5 the ADO connection will throw an exception The connection does not support MultipleActiveResultSets

Enable MARS (not that I want to) and the exception changes to

"The transaction operation cannot be performed because there are pending requests working on this transaction"

The query being executed that causes this is from the IContainerService.SetInitialOrder()

var contanerContentItemsQuery = _session.QueryIndex<ContentItemIndex>(x => x.ContentType == contentType && (x.Published || x.Latest));
            var containerContentItems = await contanerContentItemsQuery.ListAsync();

Of note, this is not using the Database Content Definition Store. It's using the File Definition Store.
So there is no other queries going to the database, nor is there any _session.Save() occuring.

I think the issue is happening because of async void calls in the DefaultContentDefinitionDisplayManager

This is the method causing issues

        public async Task<dynamic> UpdateTypePartEditorAsync(ContentTypePartDefinition contentTypePartDefinition, IUpdateModel updater, string groupId = "")
        {
            if (contentTypePartDefinition == null)
            {
                throw new ArgumentNullException(nameof(contentTypePartDefinition));
            }

            dynamic typePartDefinitionShape = await CreateContentShapeAsync("ContentTypePartDefinition_Edit");
            var layout = await _layoutAccessor.GetLayoutAsync();

            _contentDefinitionManager.AlterTypeDefinition(contentTypePartDefinition.ContentTypeDefinition.Name, typeBuilder =>
            {
                typeBuilder.WithPart(contentTypePartDefinition.Name, async typePartBuilder =>
                {
                    typePartDefinitionShape.ContentPart = contentTypePartDefinition;

                    var partContext = new UpdateTypePartEditorContext(
                        typePartBuilder,
                        typePartDefinitionShape,
                        groupId,
                        false,
                        _shapeFactory,
                        layout,
                        updater
                    );

                    //  BindPlacementAsync(partContext).GetAwaiter().GetResult();

                    //  _handlers.InvokeAsync((handler, contentTypePartDefinition, partContext) => handler.UpdateTypePartEditorAsync(contentTypePartDefinition, partContext), contentTypePartDefinition, partContext, _logger).GetAwaiter().GetResult();

                    await  BindPlacementAsync(partContext);

                    await _handlers.InvokeAsync((handler, contentTypePartDefinition, partContext) => handler.UpdateTypePartEditorAsync(contentTypePartDefinition, partContext), contentTypePartDefinition, partContext, _logger);
                });
            });

            return typePartDefinitionShape;
        }

Swap the async / await on the typeBuilder.WithPart for a GetAwaiter().GetResult() and the issue is resolved.

Problem is I think the async voids are just heading off into never never land, and not completing before the session is trying to Commit.

That's why I say 4 times out of 5.

If you put it in the debugger (i.e. give it a chance to complete the async database queries), it never fails.

The ContentDefinitionDisplayManager is actually peppered already with .GetAwaiter().GetResult() calls on some of the other methods, and async await void on some of the others.

I guess it's not a heavy usage api, so .GetAwaiter().GetResult() is probably better than breaking the api by turning it async, but there's probably a nicer way to fix it.

Any ideas @jtkech ?

[jtkech]

@deanmarcussen

Yes, we should use .GetAwaiter().GetResult() when in the lambda of an .AlterSomething() as it is not related to a Func<> returning a Task, but an Action<> parameter that can't be async. Hmm, i already fixed the same kind of issue, should be as it is already done line 97 and 155, but we missed this one line 216 and also another one line 280