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.

Sign up for GitHub

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

Jump to bottom

Do not define finalizers in types derived from MemoryManager<T> #33768

Closed
terrajobst opened this issue Mar 19, 2020 · 3 comments · Fixed by dotnet/roslyn-analyzers#3461
Closed

Do not define finalizers in types derived from MemoryManager<T> #33768

terrajobst opened this issue Mar 19, 2020 · 3 comments · Fixed by dotnet/roslyn-analyzers#3461
Assignees
@buyaa-n
Labels
api-approved API was approved in API review, it can be implemented area-System.Buffers code-analyzer Marks an issue that suggests a Roslyn analyzer
Milestone
5.0.0

Comments

@terrajobst
Copy link
Member

Adding a finalizer to a type derived MemoryManager<T> is likely an indication of a bug, as it suggests a native resource that could have been handed out in a Span<T> is getting cleaned up and potentially while it's still in use by the Span<T>.

Category: Reliability
@terrajobst terrajobst added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Buffers code-analyzer Marks an issue that suggests a Roslyn analyzer labels Mar 19, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Mar 19, 2020
@dotnet dotnet deleted a comment from Dotnet-GitSync-Bot Mar 19, 2020
@jeffhandley jeffhandley added this to the 5.0 milestone Mar 20, 2020
@jeffhandley
Copy link
Member

Estimates:

  • Analyzer: Small
  • Fixer: Not Applicable

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed untriaged New issue has not been triaged by the area owner api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Mar 21, 2020
@buyaa-n
Copy link
Contributor

buyaa-n commented Mar 24, 2020 •
edited
Loading

Test scenarios:

  1. Type not derived from MemoryManager with or without finalizer should be OK
  2. Type derived from MemoryManager without finalizer should be OK
  3. Class directly derived from MemoryManager having finalizer should warn
  4. Class indirectly derived from MemoryManager having finalizer should warn

@buyaa-n
Copy link
Contributor

buyaa-n commented Mar 24, 2020

@bartonjs @terrajobst I have used warning level for severity and true for isEnabledByDefault please let me know if that is correct

This was referenced Mar 30, 2020
Closed
Merged
@GrabYourPitchforks GrabYourPitchforks mentioned this issue Jun 1, 2020
Open
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@buyaa-n buyaa-n

Labels
api-approved API was approved in API review, it can be implemented area-System.Buffers code-analyzer Marks an issue that suggests a Roslyn analyzer
Projects
None yet
Milestone
5.0.0
4 participants
@jeffhandley @terrajobst @Dotnet-GitSync-Bot @buyaa-n