Critical: Umbraco Commerce crashes after upgrading to .NET 8.0.10 and can't place any orders

[arknu]

Describe the bug
After upgrading to .NET 8.0.10, Umbraco Commerce crashes

I have debugged the issues and traced it to the MemoryExtensions class. This class does some reflection on internal state of the .NET MemoryCache class.

The is the (decompiled) code in question:

private static readonly Lazy<Func<object, IDictionary>> GetEntries7 = new Lazy<Func<object, IDictionary>>(() => CreateGetter<object, IDictionary>(typeof(MemoryCache).GetNestedType("CoherentState", BindingFlags.NonPublic).GetField("_entries", BindingFlags.Instance | BindingFlags.NonPublic)));

Unfortunately, the implementation of the internal CoherentState class has changed between .NET 8.0.8 and 8.0.10.
Here is 8.0.8:

 private sealed class CoherentState
        {
            internal ConcurrentDictionary<object, CacheEntry> _entries = new ConcurrentDictionary<object, CacheEntry>();
            internal long _cacheSize;

And here is 8.0.10:

private sealed class CoherentState
        {
            private readonly ConcurrentDictionary<string, CacheEntry> _stringEntries = new ConcurrentDictionary<string, CacheEntry>(StringKeyComparer.Instance);
            private readonly ConcurrentDictionary<object, CacheEntry> _nonStringEntries = new ConcurrentDictionary<object, CacheEntry>();
            internal long _cacheSize;

Notice how the _entries variable has been split into two in 8.0.10.

And this is an excellent example of why taking a dependency on internal .NET state like this is very dangerous. You need to fix this ASAP! We are now vulnerable to security issue because we had to downgrade.

This is the full stacktrace of the NullReferenceException being thrown:

System.NullReferenceException: Object reference not set to an instance of an object.
   at Umbraco.Commerce.Infrastructure.Resiliency.PollyExecutionStrategyBase.Execute[TResult](Func`1 operation, Func`1 verifySucceeded)
   at Umbraco.Commerce.Core.UmbracoCommerceUnitOfWorkProvider.Execute[T](Boolean autoComplete, Func`2 action)
   at Umbraco.Commerce.Core.Services.OrderService.PerformGetState(Guid id)
   at Umbraco.Commerce.Core.Cache.DefaultEntityStatePolicyCache`2.Get(TId id, Func`2 performGet, Func`2 performGetAll)
   at Umbraco.Commerce.Core.Services.OrderService.GetOrderState(Guid id)
   at Umbraco.Commerce.Core.Services.OrderService.GetOrder(Guid id)
   at Umbraco.Commerce.Core.Api.CoreUmbracoCommerceApi.GetOrder(Guid orderId)
   at TITSAM.Core.Brobygger.Services.BrobyggerTicketDetailsProvider.CanProvideDetails(TicketToken token)
   at TITSAM.Core.Web.Controllers.TicketviewController.<>c__DisplayClass3_0.<Get>b__0(ITicketDetailProvider x)
   at System.Linq.Enumerable.TryGetFirst[TSource](IEnumerable`1 source, Func`2 predicate, Boolean& found)
   at TITSAM.Core.Web.Controllers.TicketviewController.Get(Guid token)
   at lambda_method1216(Closure, Object)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.AwaitableObjectResultExecutor.Execute(ActionContext actionContext, IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeActionMethodAsync>g__Awaited|12_0(ControllerActionInvoker invoker, ValueTask`1 actionResultValueTask)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeNextActionFilterAsync>g__Awaited|10_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Rethrow(ActionExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeInnerFilterAsync>g__Awaited|13_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResourceFilter>g__Awaited|25_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResourceExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeFilterPipelineAsync>g__Awaited|20_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Umbraco.Cms.Web.Common.Middleware.BasicAuthenticationMiddleware.InvokeAsync(HttpContext context, RequestDelegate next)
   at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.InterfaceMiddlewareBinder.<>c__DisplayClass2_0.<<CreateMiddleware>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Umbraco.Cms.Web.BackOffice.Middleware.BackOfficeExternalLoginProviderErrorMiddleware.InvokeAsync(HttpContext context, RequestDelegate next)
   at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.InterfaceMiddlewareBinder.<>c__DisplayClass2_0.<<CreateMiddleware>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Session.SessionMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Session.SessionMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Localization.RequestLocalizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at StackExchange.Profiling.MiniProfilerMiddleware.Invoke(HttpContext context) in C:\projects\dotnet\src\MiniProfiler.AspNetCore\MiniProfilerMiddleware.cs:line 112
   at Umbraco.Cms.Web.Common.Middleware.UmbracoRequestMiddleware.InvokeAsync(HttpContext context, RequestDelegate next)
   at Umbraco.Cms.Web.Common.Middleware.UmbracoRequestMiddleware.InvokeAsync(HttpContext context, RequestDelegate next)
   at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.InterfaceMiddlewareBinder.<>c__DisplayClass2_0.<<CreateMiddleware>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Umbraco.Cms.Web.Common.Middleware.PreviewAuthenticationMiddleware.InvokeAsync(HttpContext context, RequestDelegate next)
   at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.InterfaceMiddlewareBinder.<>c__DisplayClass2_0.<<CreateMiddleware>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Umbraco.Cms.Web.Common.Middleware.UmbracoRequestLoggingMiddleware.InvokeAsync(HttpContext context, RequestDelegate next)
   at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.InterfaceMiddlewareBinder.<>c__DisplayClass2_0.<<CreateMiddleware>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at SixLabors.ImageSharp.Web.Middleware.ImageSharpMiddleware.Invoke(HttpContext httpContext, Boolean retry)
   at Umbraco.Commerce.Cms.Web.Mvc.UmbracoCommerceRequestBufferingMiddleware.InvokeAsync(HttpContext context, RequestDelegate next)
   at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.InterfaceMiddlewareBinder.<>c__DisplayClass2_0.<<CreateMiddleware>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at TITSAM.Core.ServiceExtensions.<>c.<<UseTicketView>b__0_0>d.MoveNext()
--- End of stack trace from previous location ---
   at TITSAM.Core.Web.Middleware.MaintenanceModeMiddleware.InvokeAsync(HttpContext context, IOptionsSnapshot`1 settings)
   at Microsoft.AspNetCore.Server.IIS.Core.IISHttpContextOfT`1.ProcessRequestAsync()

The exact stack trace is really not that relevant, though, as it will crash doing pretty much anything, as most operations go through the EntityCache, which again uses these broken MemoryCache extensions.

Umbraco Commerce version:
13.1.6

---

This item has been added to our backlog AB#44922

[tkf-ordeno]

Ran into the same issue today after updating Visual Studio.

Simple repro is running the demo store with the SDK 8.0.403 installed and then use "Add to cart".

[arknu]

@tkf-ordeno Good to hear I'm not alone. I'm frankly astonished to see that no one from Umbraco has even reacted to this issue yet. Commerce is completely broken by a servicing update and they just don't care...

[AndyButland]

It's been noted, don't worry... as it's closed source I have realised you don't see a link to it, but there's a PR prepared to look to resolve this. We need to test it carefully, but will get it out as a patch release as soon as we can.

[arknu]

@AndyButland Thanks! Any ETA on an update?

[AndyButland]

Nothing further yet than what I just posted above, but as I say, as soon as we have verified the fix we'll get a patch release out.

[acoumb]

Hi @arknu , @tkf-ordeno ,

Could you test if the issue is fixed with this preview version (Umbraco.Commerce.13.1.7--preview.1.ge33f7ea.nupkg) from the MyGet feed?

Thanks

[arknu]

@acoumb Thanks! I have tested this preview version and it appears that everything is working normally again.

[tkf-ordeno]

@acoumb I'm using 14.1, but it is the same issue so I guess you need to apply the same patch to your internal v14 branch afterwards 🙂

[asawyermarathon]

I'm running 14.1.1 and just ran into this. I was baffled and thought I broke something. Local environment crashes, dev environment still works. I must have updated my local environment when I restarted my computer this morning.
Waiting on a patch or even a preview version for 14 so that I can continue development.

[ctolkien]

This servicing update is in the aspnet:8.0 docker images (as of yesterday)

[acoumb]

Hi all,

Issue is now addressed with patch versions 13.1.7 and 14.1.2 available on NuGet.

Updates to the demo store will follow shortly.

[arknu]

@acoumb I have now rolled the fix out to production, as well as reinstalling .NET 8.0.10 and I can confirm that everything is working again.

Thank you for fixing this quickly.

[TRiV07]

@acoumb thank you for your fix, but we still have issues with this update. The issue appears not immediately, but after some time.
I looked on updated EntityCache.cs decomplication and _keys list is not updating when cached item expired. As a result, GetAll can return nulls.
Could you look at it again, please?

[acoumb]

Hi @TRiV07 , thank you for bringing this to our attention. I will test and try to replicate your use case.

Anything specific that could help me replicate it easily? Thanks

[TRiV07]

Hi @acoumb
On my local environment I reproduces an issue in the next way: I added item to basket, waited some time (it was around 30 minutes) refreshed the basket page, and issue appeared. I don't know what the cache expiration timeout for orders, but you actually need to wait a little more than expiration. Then the order will be removed from the MemoryCache, but still exists in _keys.

[ctolkien]

Confirming that we are seeing a similar issue whereby after a user adds something to their cart and waits some time, requests are failing with a NRE.

[marnixvanwijkthevalley]

We are also experiencing this issue, also on .net 8.0.8. is there a combination of .net 8 that does work? failing carts is bad for business

[jveer]

Hi @acoumb / @AndyButland, any update on this issue? We are currently unable to process any orders.

[acoumb]

Hi, I am preparing a patch release to the MyGet feed for you to test. Will be back with details very soon.

[acoumb]

Would a patch version for Umbraco 13 suffice, or should I prepare one for 14 as well?

[jveer]

Would a patch version for Umbraco 13 suffice, or should I prepare one for 14 as well?

For us, this applies to Umbraco 13.

[acoumb]

We are still running some tests before releasing the preview.

[acoumb]

Could you try with this preview version from MyGet: Umbraco.Commerce.13.1.8--preview.3.gc7f0d71?

[jveer]

@acoumb it does seem to work faster on our cart page, and I don't see concurrent errors (which we had before, when quickly updating multiple quantities).
I'll keep the site running for a bit to ensure the caches are working correctly, I'll let you know in a bit.

Update:
Unfortunately, it's happening again. Much quicker now as well (5-6 minutes after startup):

System.ArgumentNullException: Value cannot be null. (Parameter 'key')
   at Umbraco.Commerce.Infrastructure.Resiliency.PollyExecutionStrategyBase.Execute[TResult](Func`1 operation, Func`1 verifySucceeded)
   at Umbraco.Commerce.Core.UmbracoCommerceUnitOfWorkProvider.Execute[T](Boolean autoComplete, Func`2 action)
   at Umbraco.Commerce.Core.Session.SessionManager.GetCurrentOrder(Guid storeId, String customerReference)

I see this line was added in the ChangeTrackingEntityCache:
if (this._globalCache.Keys.Contains<string>(key) && this._globalCache.Get(key) != null)

However, the Contains check returns true, but the Get(key) throws the exception. Although correctly checking the value of key should prevent the error.. I also think this is just a workaround, as the source of the error (why key is null in the first place) won't be solved with this?

[acoumb]

Could you try again with this preview version Umbraco.Commerce.13.1.8--preview.4.gaa4c115 and provide us with a couple of details (we cannot replicate the issue on our side so far)?

• installed version of Commerce
  
• are you testing this with the demo store, or a different implementation?

Thanks

[jveer]

So far it seems to be working (10-15 minutes in), I'll keep you posted.

The installed version of Umbraco Commerce is indeed saying v13.1.8:

And for testing I'm using the custom implementation that we have for our client. Which has been working without any issues before.

[mattbrailsford]

@jveer can you try with the demo store as we need to rule out custom code https://github.com/umbraco/Umbraco.Commerce.DemoStore

[jveer]

@jveer can you try with the demo store as we need to rule out custom code https://github.com/umbraco/Umbraco.Commerce.DemoStore

We're now over 30 minutes in, and the site is still working. I can set up the DemoStore, but I would first need to verify that the issue is happening on 13.1.7 (waiting 20 minutes+), then with the new patch fix. This will take some time, and I can't promise I'm able to finish that today.

I'll get back to you when I have the results of testing.

[mattbrailsford]

No problem. We appreciate your assistance testing this @jveer

[jveer]

@mattbrailsford I can't reproduce this issue with the DemoStore either with version 13.1.7 (after 30 minutes of running).

With the latest patch fix, I also didn't face the issue anymore on our custom implementation. Not sure what was changed, but it seems to have helped.

I will do some testing to see if it's related to our custom ProductCalculator, which has some custom logic for the TaxRate.

[jveer]

@mattbrailsford disabling the calculators etc.. didn't change much.

However, using the version before the last patch (Umbraco.Commerce.13.1.8--preview.3.gc7f0d71), I noticed in my incognito browser starting a new cart.. that one kept working while the "old" cart was broken.

With the debugger attached, I see that the broken cart uses the uc_ec_FrozenPrice_ cache keys (with 1 null value):

While the working cart uses the uc_ec_OrderState_ cache keys:

Not sure if that gives you an idea of where I should look at for debugging?

[TRiV07]

Hi @acoumb

I tested 13.1.8--preview.4.gaa4c115 and can confirm that cache expired issue (after waiting 30 minutes) does not appears now.

[acoumb]

Sounds good, I'll leave it for a bit in monitoring by others who experience this, then proceed with the update.

Thank you

[mattbrailsford]

That’s excellent news. Fingers crossed others can confirm the same 🤞🏻

[acoumb]

Version 13.1.8 is now available on NuGet. The patch for Umbraco 14 will follow soon.

Thank you all for the effort.

[marnixvanwijkthevalley]

We have released this version to production, no more broken carts.
Thanks all for the dedication to get this resolved!

[mattbrailsford]

Closing this issue now, but thank you everyone for your assistance in getting this one sorted. 🙏

[mattbrailsford]

For anyone reading this issue, it took us a few attempts to get to the bottom of this. It’s advised to upgrade to 13.1.10 or 14.1.5. Sorry for any additional upgrade work you need to do.

[mattbrailsford]

Just a heads up, 13.1.12 and 14.1.17 has been released which fixes some further regressions connected to the same issue so you may wish to upgrade when you have the chance.

We now have all the related code under unit test so fingers crossed this is the last we hear of this issue.

[marlonscloud]

This issue is STILL present in 13.1.17 if you set "isRecurring" to true as described here https://docs.umbraco.com/umbraco-commerce-payment-providers/stripe/how-to-guides/processing-subscription-payments

[mattbrailsford]

@marlonscloud this is not the same and your issue is being addressed in issue #614