-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Comments
/cc @terrajobst |
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. |
Would be nice to have some kind of tool anyway that would give compat warnings if not the compiler. |
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? |
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.
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/ |
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. |
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? |
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
@terrajobst is working on a plan for obsoleting APIs like this. |
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. |
@PhilipHassialis if you like you could offer a PR to clarify it in https://github.com/dotnet/dotnet-api-docs/blob/master/xml/System.Threading/Thread.xml#L524 |
@terrajobst, how should issues like this feed into your obsoletion/tooling plans? |
This has been done for .NET 5 so this can be closed. Thanks @jeffhandley for pointing this out to me! |
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 incatch(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 withIsError=true
.The text was updated successfully, but these errors were encountered: