Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request Thread.Abort to be marked [Obsolete] #28664

Closed
lcsondes opened this issue Feb 11, 2019 · 13 comments
Closed

Request Thread.Abort to be marked [Obsolete] #28664

lcsondes opened this issue Feb 11, 2019 · 13 comments
Assignees
Milestone

Comments

@lcsondes
Copy link

We're in the middle of porting a legacy Framework project to Core, and Thread.Abort() was a sore point. It compiles without a single warning, and at runtime it just throws a PNSE. Given the legacy status of our code cleanups tend to be wrapped in catch(Exception) so this went unnoticed until we saw the memory leaks and traced them back. That one is entirely on us, of course.

If Abort doesn't work and it is planned that it never will (which is my understanding) then I think it at the very least should be marked as [Obsolete], maybe even with IsError=true.

@benaadams
Copy link
Member

/cc @terrajobst

@kouvel
Copy link
Member

kouvel commented Feb 23, 2019

These APIs are part of netstandard, so perhaps the warning should only occur when compiling for netcoreapp and not for netstandard, such that when compiling for runtimes that may support it would not warn. Not sure how exactly that would work but it may be possible.

@kouvel
Copy link
Member

kouvel commented Feb 23, 2019

Would be nice to have some kind of tool anyway that would give compat warnings if not the compiler.

@khellang
Copy link
Member

@Symbai
Copy link

Symbai commented Nov 18, 2019

This should definitely throw an error. It is not even documented in the breaking change docs: https://docs.microsoft.com/en-us/dotnet/core/compatibility/framework-core

What's the equivalent on .NET Core?

@kouvel
Copy link
Member

kouvel commented Nov 18, 2019

What's the equivalent on .NET Core?

There's no reasonable equivalent in .NET Core and it's not planned to be supported there. https://github.com/dotnet/coreclr/issues/20705#issuecomment-443433749 and other comments in that issue describe the reasons for it.

This should definitely throw an error.

Since this is a runtime-specific difference, when compiling for netstandard perhaps it could be a compat warning since they may run on runtimes that allow thread abort. When compiling for netcoreapp it probably should be an error. I don't think there's a way to do that currently.

For now the .NET API Analyzer might be useful: https://docs.microsoft.com/en-us/dotnet/core/porting/

@danmoseley
Copy link
Member

As @khellang points out, https://github.com/dotnet/platform-compat was intended to flag this kind of defunct API. It's languished a bit, I mentioned this in https://github.com/dotnet/corefx/issues/40739 to see what we will do.

@PhilipHassialis
Copy link

Is there any recent development on this? Thread.Abort() can still be typed without any kind of intellisense from the editor that it is obsolete (.NET Core 3.1 here). Souldn't the Abort becoming obsolete and/or moving out of .Net Core be mentioned in the documentation?

@jkotas
Copy link
Member

jkotas commented Jan 1, 2020

mentioned in the documentation

It is mentioned in the documentation. Look for ".NET Core only: This member is not supported." in https://docs.microsoft.com/en-us/dotnet/api/system.threading.thread.abort?view=netcore-3.1

obsolete

@terrajobst is working on a plan for obsoleting APIs like this.

@PhilipHassialis
Copy link

Ah thanks a bunch - unfortunately didn't catch this one, saw the method there along with the example and didn't catch this one. A million apologies beforehand, unfortunately most of the C# courses out there keep mentioning the method, oblivious to the changes that Core brings to the APIs. As an adage, could the documentation team kindly put something in the titles or the first paragraph or somesuch of the APIs to be deprecated so that users can immediately see that these methods may be there but will not work?

Thanks so much in advance, happy new year and once again apologies for my quite rushed questioning.

@danmoseley
Copy link
Member

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 5.0 milestone Feb 1, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@stephentoub
Copy link
Member

@terrajobst, how should issues like this feed into your obsoletion/tooling plans?

@stephentoub stephentoub removed the untriaged New issue has not been triaged by the area owner label Feb 25, 2020
@kouvel kouvel modified the milestones: 5.0.0, 6.0.0 Jul 22, 2020
@terrajobst
Copy link
Member

terrajobst commented Jan 8, 2021

This has been done for .NET 5 so this can be closed. Thanks @jeffhandley for pointing this out to me!

@ghost ghost locked as resolved and limited conversation to collaborators Feb 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests