CTR.Token throwing an ObjectDisposedException - Never merged to CoreCLR 2.1

[Casper-Olsen]

Description

In the old dotnet/coreclr repository the following error was fixed in CoreCLR 2.2, but never in CoreCLR 2.1. dotnet/coreclr#21417 (Fix CancellationTokenRegistration.Token after CTS.Dispose)

The PR for CoreCLR 2.1 was unfortunately closed before being merged: dotnet/coreclr#21416. With the following comment dotnet/coreclr#21417 (comment) :

Approved for 2.2.1.
ASP.NET should validate if this fixes their issue.
We should wait for any 2.1 fix -- close the issue.

It looks like the code should have been merged, after ASP.NET validated that it fixed their issue.
Now i don't know whether it didn't fix their issue - and for that reason has not been merged - or if its simply been forgotten.

We are experiencing the error quite frequently in our production environments. When it occurs our entire application server crashes and a restart is required.
The error occurs during our login flow, when the browser is fetching files from the server.

Is there any possibility that the fix could be merged (if it actually fixed the problem ASP.NET was experiencing) to CoreCLR 2.1 and subsequently released in a future version of the .NET Core 2.1 runtime?
This would help us and our users immensely.

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label.

[stephentoub]

It looks like the code should have been merged,

It was intentionally not merged. Those were two separate statements. It was saying that the fix for 2.2 should be validated with ASP.NET, and that the one for 2.1 should be closed as there wasn't enough feedback to justify it.

[Casper-Olsen]

Okay, fair enough - I misunderstood.
Is there any chance it could be added/merged to 2.1?

We are not in a position right now, where we have the resouces to upgrade to .NET Core 3.1.
We are expecting to use .NET Core 2.1 for the rest of the year atleast, and move to .NET Core 3.1 some time next year, before .NET Core 2.1 End Of Support (August 21, 2021).

Another possibility I have considered is temporarily moving to .NET Core 2.2, but I would rather like to avoid that considering its December 23, 2019 End Of Support.

[stephentoub]

@danmosemsft ?

[danmoseley]

@Casper-Olsen our threshold for back porting to 2.1 is now very high. This is partly because we want to minimize the risk of breaking any app currently using it : stability is a major reason for an app to be using 2.1. We would normally be looking for evidence of widespread significant impact, a pattern of requests coming through product support channels, etc.

Could you say a little more about the difficulty of moving to 3.1 for you?

[Casper-Olsen]

@danmosemsft last year we spent quite at bit of time moving our entire application server codebase from .NET to .NET Core, and we are now in a period of stabilization in terms of the technology we use.

Our resource allocation for any major technology changes with regards to our application server - and I would consider a move to .NET Core 3.1 major - is at the current time quite low.

I completely understand your need to minimize risk in 2.1 at this time and if we are the only one with this problem the risk definitely does not outweigh the reward. We also benefit quite a lot from the stability of .NET Core :)

[ghost]

Tagging subscribers to this area: @tarekgh
Notify danmosemsft if you want to be subscribed.

[danmoseley]

Thanks for the information. Based on your commentary I will close this and we will reconsider if it turns out it is having more widespread impact.

[GalaxyNetworks]

It worries me that you won't fix 2.1. An LTS version! This is exactly an issue that needs fixing if stability is what you want.
I too have had this problem. My mistake was to comment on the archived repository.

Considering how small the change is, it seems rather odd to leave such a change out of an LTS version. That's like saying no body is using 2.1 so why support it. Or just update to a newer version... But why then have an LTS version if there's no support.

It may take the rest of the year to reach 3.1. In the meantime our servers are crashing and clients are getting mad! I can only apologize to them, mitigate the issues as best as possible and then blame you for not updating an LTS version!

Updating to 2.2 wouldn't be hard for me, but I'd rather not as you no longer support that version.
Updating to 3.1 is not that easy. Not only are there lots of breaking changes. Like with the HostBuilder not supporting dependency injection in the Startup class. I can't seem to find a golden rule for how you'd register and configure your services when you can't inject stuff to the Startup class.

• How should I configure DataProtection?
• How should I provide a signing certificate for the IdentityServer?
• How should I configure authentication, cookies and all that?
  All necessary information for this is stored in singleton classes that gets dependency injected into Startup. Well... that needs to change now.

There doesn't seem to be a common way to do all this. Some of the functions support late configure where I can use dependency injection. Others don't! I could use IConfiguration for some of the information, but unless I'm mistaken, that only holds strings. Should I really convert my certificate to a string?

Entity Framework Core now throws a runtime exception if it fails to evaluate a query... I like the fact that it doesn't simply run it all client-side. Queries needs to be written correctly, buy why can't it analyze the query at compile time? Runtime errors are not acceptable! Even with lots of test, we still need to go through the entire application and test everything.

Last but not least... Getting clients to update is not an easy task, especially after major changes like a framework upgrade.
Right now it really feels like .NET Core aren't ready for large applications running in a production environment.

[danmoseley]

But why then have an LTS version if there's no support.

@GalaxyNetworks it is fully supported but as mentioned above, one of the reasons folks use LTS is to be supported with minimal churn. The cost of the port itself to us is not material, it's the risk of somehow breaking someone who's app was working fine in deployment for 2+ years and wants to be able to continue to take security fixes. With every change like this it is a balance.

We haven't gotten much feedback about pain migrating to 3.1, but I hear you and understand your issues are real. It seems that these are issues that the ASP.NET and EF team can better answer -- can I help you engage with them in their repos, if you have not already done so?

[danmoseley]

Having said that, talking to @stephentoub , your feedback is another +1 for porting it. We will take it for review.

[GalaxyNetworks]

I've looked and looked and only found this recent issue for 2.1 so we may not be many who's suffering. I haven't seen this issue during development so seeing our production servers crash was a surprise. This gets worse with more customers getting migrated to the new servers and that's why I decided to comment on the issue even though it was closed.

I might have misunderstood what gets updated with LTS and I completely understand your fear for breaking stuff. That's exactly the reason for why we're still not on 3.1. I have a branch for 3.1, and it all seems to work. We still need to fix some badly written EF queries, and due to the scale of the upgrade, we need to test everything, and then we need to upgrade customers. Our small cloud services are easy to upgrade, but our larger on-prem solutions are a pain!

I guess it's always easier to blame the other guy :) It just seems like such a small change that shouldn't break anything, but of course that's easy for me to say.

I really like the changes to EF Core. Our issues are due to badly written/misunderstood queries that should run on the server but can't. It just makes it harder to fix when they fail at runtime and not compile time, but then again this forces us to go through them all, make sure they're all tested and then optimize them for 3.1. It just takes time, but I think it's worth it!

I really hope you'll reevaluate your decision and fix 2.1 as well. I know it's much to ask for, but it really hurts seeing our servers crash and not being able to easily fix it. Had we just known about this issue before upgrading from .Net Framework to .Net Core 2.1 then we would have waited for 3.1. But then again, there's always a risk. :)

[danmoseley]

@GalaxyNetworks you probably know this, but to share feedback or get help with EF Core and ASP.NET Core the best thing to do is open issues in https://github.com/dotnet/aspnetcore and https://github.com/dotnet/efcore. Folks in this repo aren't experts in it. We do want you to be successful but that's the best way to share your feedback.

Thank you for giving more context on this issue. We'll see what we can do.

[tarekgh]

This is servicing port request so I changed the milestone to 6.0 as we don't have 2.x milestones and this is not 5.0 issue.

[threenub]

@tarekgh Does this mean a fix is in fact coming to netcore 2.1?

[tarekgh]

@threenub I have just changed the milestone as this is not 5.0 issue. I'll let @danmosemsft comment on the servicing part as he is the one activated this issue so I assumed was looking if we can port it.

[danmoseley]

This was in 2.1.21 which released a few days ago.

[threenub]

Fantastic. Thanks for the update!